Allow override for SoftwareSerial timer ISR priority

[sjasonsmith]

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.

1. Replace TIMER_SERIAL constant with SWSERIAL_TIMER_NUM.

   • Prefix clearly communicates that this constant impacts SoftwareSerial.
   • Providing timer number instead of name simplifies construction of interrupt name at build time.
     • This change could be skipped if there is a good way to determine an interrupt number from a timer base address at runtime (Since TIMx is actually a macro to the base address).
   • #error can be used to communicate change if TIMER_SERIAL is defined

2. Add SWSERIAL_TIMER_INT_PRIO to override interrupt priority

   • Only change the priority if the constant is defined, else use default.
   • Alternatively, default to highest interrupt priority for best bit-timing performance, and allow users to downgrade priority with constant.

3. Expose additional macros with SWSERIAL prefix, allowing users to customize behavior if needed:

   • SWSERIAL_FORCE_BAUD_RATE (Currently override-able as FORCE_BAUD_RATE)
   • SWSERIAL_OVERSAMPLE
   • SWSERIAL_HALFDUPLEX_DELAY

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.

[LinoBarreca]

...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
for example SoftwareSerial(timer, priority)
the oversample and halfduplex could be just properties that the user can set to change the object behaviour.

[fpistm]

I guess @LinoBarreca proposed the best option.

[sjasonsmith]

...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

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:

1. The same SoftwareSerial interfaces exists on multiple platforms. Adding capabilities to the interface would require applying the same changes for AVR, 176x, etc. If capabilities were added only for one platform, generic code would be unlikely to use them.

2. Runtime alternatives would not be accessible if SoftwareSerial is used indirectly through other libraries, unless those libraries in-turn exposed them to users. Marlin using SoftwareSerial indirectly through TMCStepper is an example. The #define option allows behavior to be altered without requiring changes to library code.

[fpistm]

That make sense. Do not hesitate to provide a PR

[sjasonsmith]

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 😃

[sjasonsmith]

I could add a static function like SoftwareSerial::setTimerIntPriority to change the priority at runtime. That might be easier to use from Marlin anyway.

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.

[LinoBarreca]

you are right about "existing interfaces".
but you are keeping the interface as is, actually.
You keep what you have (compatibility with old code) and add new, platform specific, things.
You can't keep compatibility anyway (unless you keep it so generic that you can't unleash the features of more modern platforms) because platforms are necessarily different....
Adding something isn't breaking the interface. you still implement the interface methods. and add your own.

[LinoBarreca]

..but we agree on one thing: as is, the softwareserial, has a big problem in real life usage scenarios :D

[sjasonsmith]

..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!