-
Notifications
You must be signed in to change notification settings - Fork 1k
Allow override for SoftwareSerial timer ISR priority #751
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
Comments
...or instead of defining them as constants (at build time) provide some methods to change the values on the instantiated object which reflects what the user wants to do. If it's simpler for you (because you don't change them once instantiated) you could provide some alternative constructors |
I guess @LinoBarreca proposed the best option. |
Thanks for the suggestion. I considered this, and it would be my preferred option if this were brand-new code rather than an existing interfaces. As it is now, I see the following problems with adding run-time options to the existing interface:
|
That make sense. Do not hesitate to provide a PR |
Of the changes I proposed, the interrupt priority is the most important aspect. I could add a static function like SoftwareSerial::setTimerIntPriority to change the priority at runtime. That might be easier to use from Marlin anyway. The only issue I have implementing that is I haven't yet found a simple way to get an interrupt number from a timer name. Changing the #define to accept a timer number instead of a name allows it to be easily derived at build-time using macros. I'll play with this today. If I come up with something I like and haven't heard more suggestions, I'll put up a PR and it can be ripped apart there 😃 |
This is contrary to my previous comment about not wanting to change the interface. In this case it seems like less of an issue, since interrupt priority is already platform-specific, and would be configured by board-specific code rather than generic library code. |
you are right about "existing interfaces". |
..but we agree on one thing: as is, the softwareserial, has a big problem in real life usage scenarios :D |
We probably agree on lots of things, but sometimes communication breaks down across text, languages, and 8 time zones! |
Fixes stm32duino#751 Signed-off-by: Frederic Pillon <[email protected]>
@sjasonsmith , @LinoBarreca |
Thanks @fpistm, that is good information. Unfortunately we are working with boards that are already built, and often do not have hardware UARTs wired to the right places. I would always choose HardwareSerial if I could. Do you know approximately when the next release will occur, which could include the Timer/SoftwareSerial fixes in #749 and #755? |
This should be in december. |
@fpistm we use a mix, actually, of soft/hard serials, full/half duplex according to the board variant (how the manufacturer actually wired the chip) and the user configuration. Good to know that, for those who have the option to use a HardwareSerial, we already have the fix if the TX/RX is the same. well done. |
Is your feature request/improvement related to a problem? Please describe.
The recently added SoftwareSerial library uses the default timer priority for software bit timing. For many processors, this defaults to preemption level 14, which will cause bit timing issues when integrating with systems using other interrupts on the system.
This became apparent when integrating SoftwareSerial with Marlin when testing the following Marlin PR:
MarlinFirmware/Marlin#15655
Describe the solution you'd like
I suggest the following changes to the SoftwareSerial library. I am willing to contribute these changes, but wanted input on whether these changes were likely to be accepted before creating a PR.
Replace TIMER_SERIAL constant with SWSERIAL_TIMER_NUM.
Add SWSERIAL_TIMER_INT_PRIO to override interrupt priority
Expose additional macros with SWSERIAL prefix, allowing users to customize behavior if needed:
Describe alternatives you've considered
Working around this in Marlin was quite cumbersome. We were able to get this to communicate by arranging other timer interrupts around the default timer interrupt level, but that solution is not ideal. It likely causes other timing issues due to changes to prioritization among hardware ISRs, etc.
I attempted to set the timer ISR priority from within Marlin, but the change did not persist. I believe that when the timer is reset during a baud rate switch (setSpeed) the timer is being restored to its default priority level.
The text was updated successfully, but these errors were encountered: