-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
STM32 - Higher serial IRQ priority #22529
Conversation
abffbbe
to
22ae09a
Compare
Should there be any consideration for USB serial, per this comment from @rhapsodyv…?
|
@sjasonsmith can you review this? |
@ldursw, your description says "Increased interrupt priority for serial communications", but your changes only reduce interrupt priorities. Can you describe the reasoning for each change you made, and how they related to serial communication? None of the interrupts you modified are the actual serial interrupts used when communicating with a host. |
I now see this comment. I think this change is a problem. The interrupts you reduced (specifically the Servo and SWSerial interrupts) are much more sensitive to timing jitter than the normal serial interrupt would be. Your changes may reduce some issues with host communication, but will worsen problems with BLTouch and TMC Serial behavior. @rhapsodyv are all these serial performance problems related to the serial refactor earlier this year? I don't recall keeping up with host I/O as a problem in the past. When we were debugging those Maple interrupt issues we were talking to displays at 500kbaud. |
Unsure USB serial use the same interrupts as raw serial... but im not familiar enough with STM32 USB for now. Is the issue on a USB serial board (no other USB chip) or with a direct UART connector ? |
How many cables of different qualities did you try? Do any of the cables have ferrite beads on them? |
I can try lowering just the step priority and see if it still works well.
The board is a MKS Robin E3. It uses hardware UART with a CH340 for serial-USB conversion. I also reproduced the issue on a Creality 4.2.2 board which also uses hardware UART and a CH340.
I have tested different cables with different lengths and quality and it doesn't make any difference. If I get the exact same commit but compile with libmaple I can print at 921600 without issues using the same cable. I'm fairly certain that this is specific to how stm32duino handles serial communication. As it uses interrupts for receiving data and Marlin also uses interrupts for other tasks some data ends up being lost when the serial interrupt can't run. I'll post a serial log showing the issue soon-ish. |
Maple also uses interrupts to service the serial port, so that alone doesn't explain the problem. Are the new priorities what were being used with Maple? I don't remember exactly how interrupts were configured in the F1 HAL, but they should have been similarly high. F4 boards tend to use native USB for serial communication, so there may be other inefficiencies in the serial path in the STM32 HAL that went unnoticed until F1 boards with external USB chips started moving over. |
@sjasonsmith and I reworked maple ISR to handle overrun sometime ago... |
Ideally the ISR would do as little as possible to unload the data, then defer additional overhead until the main loop retrieves the data. Perhaps the two platforms vary a lot in the way this is done. |
right, so your problem should be visible on my card too, same system... i just didnt do octoprint prints for now (migrated last week to STM32/ HAL) Just used the monitor for the serial, which looks good |
about that, STM32 seems to use a mixed system of UART and USART modules... (synchronous or asynchronous).. Still need to investigate in the framework code. Another thing to note... UART1 pins are faster than others, its explained in the F103 datasheet HAL_USART_MODULE_ENABLED vs HAL_UART_MODULE_ENABLED there are also flags about some callbacks :
|
In former days Marlin had direct calls to the UART handler inside the Stepper ISR to avoid potential missed bytes. We got rid of that by fixing up interrupt priorities, so this PR would be consistent with that previous solution. The main thing is that we don't want to mess up step timing, especially since some Trinamic drivers are offended by timing irregularities. All other events in Marlin are non-critical up to a delay of several milliseconds. |
i had not dug in the codes in marlin, but that if you are using usb-serial with stm32duino i.e. the default on-chip usb support usb-cdc-acm, baud rates 'doesn't matter'. usb-cdc acm does not depend on baud rates, it can go up to some 1 Mbps (1 mega bits per sec - usb full speed is 12 mbps but that normally due to all that multiplexing with your mouse, keyboard and all other pheriperials, u'd get about 1mbps on usb-serial (usb-cdc-acm)). the 'baud rate' is normally completely controlled by the host, i.e. how often and prompt it poll for data. there is one interleaving case which is that the mcu (i.e. marlin codes) is too busy and did not have the bandwidth or time to fill up that buffer, or did not have time to process that data. that one needs to be verified and proven. if it is indeed a problem. then it is more of an 'optimization' problem in marlin edit: what is more there are usb based boot loaders out there that can do away with even needing the usb-uart dongle. you can simply use the native on-chip usb device. |
Marlin was originally designed for AVR based on Grbl methodology so it runs a continuous loop, which keep the processor quite busy and the instruction pipeline constantly fed. There is no use of processor sleep or any other feature that would suspend the main thread while still processing interrupts. I'm not sure how we would refactor Marlin to make use of low power and sleep features, other than to refactor all tasks to run off of an RTOS (if they do that). |
The F1 chips require some external circuitry for the built-in USB to work properly. This is not needed for the F4. I think BigTreeTech figured out how to make it work properly and everyone else just gave up and added an external serial chip. |
thinkyhead wrote:
we can gradually transit to the more 'powerful' mcus, mcus like stm32f4 (this is quite good and is gaining momentum) and stm32f7 (superscalar) are different from the 'old' 'gcode processor' mcus. we can literally think of running a whole '3d printing os' on it. we can think of having both octopi + 'gcode processor' on it. they are different in the sense they have much more h/w resources on it and often significantly faster. since the beginning beaglebone black (arm cortex a) with satellite 'real time' smaller microcontroller has been around for now i think there is 'no hurry', this 'gcode processing' way is still very much the norm today. back on the current topic, for stm32 (and maybe some of the nxp families) that have usb on chip hardware, they should be made used of, there has been enough issues having an additional usb-uart bridge separate from the mcu, it does not makes sense to stick to the 'old' separate usb-uart bridge when native usb device h/w interfacing is available. having usb device h/w also means that other usb protocols such as hid, dfu and every other usb protocol can be used, no longer limited to just serial. usb serial (usb-cdc-acm) is the 'new com port'. that old 'rs232' metaphor never goes away. but baud rates, dtr, rts etc is no longer relevant. and those 'signals' dtr, rts are often (ab)used for different purpose |
reposted from the pr comments and extended: STM32F1 (aka libmaple core) is leaner than STM32 ('official' core), libmaple is nearly bare metal and hence lower overheads. however, libmaple is nearly strictly stm32f103 mcu only. STM32 ('official' core) strives for compatibility across the lines and hence needs to wrap around STM HAL as that's about the only way to 'cross mcu'. tweaking irq priorities may have some implications that we'd not know for now, it may cause issues with stepping or servos. try to go for boards that use usb-serial (usb-cdc-acm). stm32f103 has usb-serial as well it is mainly a hardware board design thing and it really does not make sense to use a separate usb-uart bridge to talk to stm32f103 when stm32f103 has a better native usb hardware device support. as to the guesses about the resends, that'd be difficult to diagnose. it requires some sort of 'timing measurements' about what irq fires, and which irq handlers are hogging the timeslots. it may not necessarily be serial that is hogging the timeslots. it could even be say in the main loop() where some parts of codes disable interrupts for long periods, that alone could cause arriving bytes (on stm32f103 the uart buffer is only 1 byte) to be lost. my suspicion is that sd card reading can be one of the causes if it disable interrupts to read data from sd card. i remember somewhere sd card response timeout may be as much as 250ms, possibly during a block write. that would make even 9600 baud a joke if indeed interrupts is suspended that long. on another note i've encountered some of the stm32 families e.g. perhaps a few stm32L4xx mcu, consider the more 'expensive' ones, some of them has a fifo in the uart. of course that also means an expensive chip. things like stm32f103 are perhaps the 'basic' ones where the uart simply store a byte and interrupt firmware to handle it. using dma will have its implications as well, as dma timeslot is multiplexed, this could compromise other high speed transfers say on spi etc. to figure that out it would be difficult but which is to figure out if anything say in the main loop may be disabling interrupts for long periods. that would leave a lot less time for irq handlers to run, and hence if hardware uart has maybe say the same priority as timers, between the irq handlers all trying to complete their work, perhaps by which time the next uart byte may have been lost. placing uart irq handler at higher priority could on the other hand cause the timer interrupts to lose traction and that may have rather severe implications for servos, stepping etc. in terms of 'software serial', do note that stm32 uart has a 'single wire' uart mode. that is primarily intended for smart cards. |
Does anyone know how we would go about setting up serial DMA and using that whenever it is available? |
I didnt try yet, but there are some tutorials here https://controllerstech.com/uart-transmit-in-stm32/ & https://controllerstech.com/uart-dma-with-idle-line-detection/ some framework doc here : https://github.com/stm32duino/Arduino_Core_STM32/blob/master/system/Drivers/STM32F1xx_HAL_Driver/Src/stm32f1xx_hal_uart.c#L156-L173 and another example here for our framework https://riptutorial.com/stm32/example/30042/transmit-large-amount-of-data-using-dma-and-interrupts---hal-library |
thinkyhead wrote:
this is a 'complicated' matter. the normal HardwareSerial class from the core does not do dma. dma is also 'not perfect' as there are very few dma controllers and many peripherals (e.g. adc, uart, spi, i2c, timers etc) are connected to the same dma controller. this means say if you are using both spi and uart, the dma access would be multiplexed (shared) between spi and uart. that could introduce additional complexities once more than a single peripheral wants to use dma. but yes dma is the hardware high speed access to peripherals, it is a reason SPI on stm32 can go to like 50mhz transfer speeds. i mainly use it for things like SPI lcd displays where it helps a lot. as well as occasionally spi psram. oh and yes SD cards, dma isn't natively supported in the SPI lib, mods are necessary either in SPI lib or in the SD lib. oh and that 'old' libmaple (STM32_F1) core, does dma for spi, different methods though, libmaple is nearly 'written for stm32f103c{8,b}' so that may be a clue for someone struggling with uart issues on stm32f103xx boards. just that libmaple (roger's) core is a 'community core', there is no org behind it, but that it remains one of the very popular since leaflabs created it and the community on www.stm32duino.com enhanced it. codes is nearly 'frozen' as it depends on (very few) volunteers to merge and commit the PRs. stm32 different mcu lines e.g. stm32f1xx cortex-m3, stm32f4xx cortex-m4, stm32f7xx cortex-m7 are extensive and and heavy with lots of features within those lines/often differentiated as well due to hardware differences even within the same line. some of the codes tend to be common though, but those are rather basic as like the current HardwareSerial. i think we can think in terms of 'custom extensions', i.e. those who wanted them can make those mods in their own codes as different board designs alone would change the priorities of one needing them e.g. if you have a board with native hardware usb device support e.g. for that same stm32f103xx. it is no longer an 'issue' to you since it is not on the main 'serial' line. the trouble is i'm not sure how better to cater for 'custom extensions', 'plugins', 'libraries' etc that could perhaps be 'contributed' codes. e.g. substituting Serial, HardwareSerial with another custom one with the same api is possibly one way of a customization. for the STM core, i think as we work along, we'd need to figure out some ways to do some diagnostic measurements of the interleaving timings between the 'threads'. e.g. the main thread that runs on the cpu i.e. for some of the features that is 'common' enough, maybe we'd post them as feature requests on STM core. i think there are some requests for access to 'internal' spi private class variables, which happen to be the 'lower level' spi device. those are some of the requests that are outstanding there. the access to that 'private spi device' variable is necessary to make enhancements such as dma support etc. i'd guess it is rather similar with other peripherals |
f9aa794
to
90cd1ca
Compare
f4c2f00
to
02ae11e
Compare
The posted concerns are understood, but this change seems to be harmless. So I need to take a quick poll…
|
I’m not convinced the change is harmless. As I described in my original comments, I think this could introduce new problems for Servo/BLTouch and SoftwareSerial, which are more timing sensitive than hardware serial. |
@thisiskeithb & @ellensp, have you been hearing complaints about serial reliability issues with STM32 boards using hardware serial? If there are lots of resends at That includes USB serial on almost every non-BTT board, so I'm surprised there isn't more noise about this if it is a universal problem. |
hi all, can we take this approach at least for the time being: we can document this fix in a wiki somewhere so that those interested may edit the codes themselves and try this out. A more formal study on this problem would take doing 'timing measurements' i.e. profiling for the interrupts and various functions in loop() to see where the mcu spent its time, identify possible timing collisions and to direct the fixes to the issue hot spots. one possible issue could be the latency with the ADC analogRead() itself in stm32duino official core note that incidentally i've also logged the issue with MKS-Robin-E3-E3D |
@ag88 I’m just trying to understand how widely people are seeing this problem. I very intentionally placed the SERVO and SoftwareSerial ISR priorities as high as possible, and spent MANY hours with a logic analyzer determining that was necessary. I don’t want to undo that decision without understanding the impact. This was prior to the serial refactor of 2020, and prior to switching HALs for F1 boards, although we did something similar in the Maple HAL for the same reason. Those changes helped resolve many issues with probe failures (including related mechanical crashes) and TMC connection errors. If nobody else is reporting problems after many months without merging this change, then I have to question what makes @ldursw unique that the problem was this severe. If other people are seeing this problem also, then I think we need to pull the logic analyzer back out and understand how this will impact those two other interrupts when they are preempted by the serial ISR. |
@sjasonsmith i think this issue affects particularly boards that uses a separate usb-uart converter on stm32f103 boards. that is a bad board design decision in the first place as some of them place 'lousy' usb-uart converters that could in themselves cause problems. while all the while stm32f103 has on chip native usb support (multiplexed about 1 Mbps, full speed usb bus is 12 Mbps). the fact that stm32(f103) uart doesn't have a fifo aggravate the problems on chip usb isn't exactly 'problem free' stm32duino/Arduino_Core_STM32#1399 |
@ag88 right now I am only trying to understand how many other people are encountering the same issue. When issues are severe they tend to be reported repeatedly, which has not happened here. |
on another note, i'd think we need some sort of 'HDLC' protocols while talking with the host for the gcode file, this seem to be already there with the 'OK' response that is used very much in 'flow control' edit: on a side note, i've an Ender 3 printer with the vendor's firmware and I think i'm affected by this problem. I've not replaced that firmware with a compiled marlin. However, what we are witnessing is 'progress in flux'. I'm not sure if we'd prefer to 'push' vendors to 'make better boards', but doing so could compromise existing users with 'suboptimal boards'. the notion is that it'd also make codes more maintainable rather than having 'if defs'. e.g. for those with usb-uart 'dongle', we'd have an if-def to configure priorities 1 way. Then for those that use native usb, we'd have code that use the default priorities. |
Since this was closed on my recommendation I just want to clarify that this does not mean we want to ignore communication reliability issues. If this is still a problem at reasonable baud rates we need to document details in a bug report and figure out a different way to debug and resolve the issue, without reducing the SERVO and SoftwareSerial interrupt priorities. |
i think it is quite possible that this is partly related to spin locks while waiting for data. I have sd cards that takes like several 10s of minutes to write say 100 megs of data, even if they claimed they are after all 'class 10' cards. I'd think for reading, they are likely compliant to the specs, but there can be exceptions. the other things i'd suspect could be due to the usb-uart bridge chips themselves. I've not dug into this yet. |
Actually, Marlin assiduously avoids |
i've a strange feeling that some of these things are more relevant as a 'per board' configuration / setting. my guess is we could look at board specific initialization codes (e.g. a call back at init) so that those can set interrupt priorities etc. |
Description
Increased interrupt priority for serial communications to reduce the chance of random characters being lost at higher baudrates. If checksum is enabled the firmware reliably detects the corruption and requests a resend, but no resend is better than a few resends.
I can print at 57600 with no issues using this patch. At 115200+ there are a few resends and becomes worse the higher the baudrate. At 921600 I get 120+ resends in less than a minute of printing.
This PR won't eliminate connection issues but mitigates them. To properly fix the issue a refactor in the STM32 HAL would be necessary to use DMA instead of interrupts for serial.
Requirements
Board running on STM32 HAL.
Related Issues
#21927 (comment)