Skip to content
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

EspSoftwareSerial 8.0.1: onReceive bug fix and new namespace #8869

Merged
merged 11 commits into from
Mar 11, 2023

Conversation

dok-net
Copy link
Contributor

@dok-net dok-net commented Feb 25, 2023

Fixed onReceive IRAM cache issues.
Example for power-saving with wakeup from suspend by SW serial RX event.

@d-a-v d-a-v added the alpha included in alpha release label Feb 28, 2023
@dok-net dok-net force-pushed the swserial branch 2 times, most recently from a2921f7 to c89dac8 Compare March 2, 2023 06:25
@dok-net dok-net changed the title Upcoming EspSoftwareSerial 7.0.1 EspSoftwareSerial 7.0.1: onReceive bug fix Mar 2, 2023
@dok-net dok-net marked this pull request as ready for review March 2, 2023 06:50
@dok-net dok-net changed the title EspSoftwareSerial 7.0.1: onReceive bug fix EspSoftwareSerial <strike>7.0.1</strike>8.0.0: onReceive bug fix Mar 4, 2023
@dok-net dok-net changed the title EspSoftwareSerial <strike>7.0.1</strike>8.0.0: onReceive bug fix EspSoftwareSerial 8.0.0: onReceive bug fix and new namespace Mar 4, 2023
@dok-net
Copy link
Contributor Author

dok-net commented Mar 4, 2023

In order to be able to move forward with the development of this lib, like implementing issue resolutions for strapping pins usage as serial pins, the code is refactored to use the namespace SoftwareSerial, and rename the class SoftwareSerial to UART.
@mcspr There is a conflict with ESP32 Core's outmoded use of #defines for the SERIAL_8N1 etc; I have posted a PR espressif/arduino-esp32#7926 there, but you never know if and when... Perhaps the SoftwareSerial::SERIAL_8N1 should be renamed to SoftwareSerial::CONFIG_8N1, etc. What do you think? It's all a breaking change anyway, but there's no need to make it any harder to seek&replace than necessary. The rename to UART is in the same category, do you think that's still idiomatic?

@mcspr
Copy link
Collaborator

mcspr commented Mar 4, 2023

As previously stated, I'd be very wary with simply dropping stuff without providing any backwards compatibility.

Suppose we have softwareserial::SoftwareSerial, global namespace then has using SoftwareSerial = softwareserial::SoftwareSerial;

Same goes with config; using Foo = softwareserial::non_class_enum; would import things into global ns, see our WiFi overrides for lwip tcp state but enum still has to be prefixed with Foo::. using softwareserial::enum::A;, using softwareserial::enum::B; does avoid it, though.
Or, another option is to simply have constexpr ints, then constexpr auto SWCONFIG_8N1 = softwareserial::.*::Whatever8N1;.

(claiming SoftwareSerial for namespace feels weird tbh)

@dok-net
Copy link
Contributor Author

dok-net commented Mar 4, 2023

@mcspr You kind of mostly told me what I was hoping to hear :-) I've added the using to map class SoftwareSerial to the renamed UART. I've renamed the namespace to EspSoftwareSerial. Upper case or no, opinions vary, even I use in turn. About the other identifiers, I have renamed the enums from SWSERIAL_8N1 etc. to plain SERIAL_8N1, just must have missed that. I expect it's not used that often, so our users are invited to update their code if they do.

@mcspr
Copy link
Collaborator

mcspr commented Mar 4, 2023

Any name should be fine, just not the old class one.
Why no SWSERIAL_8N1 = EspSoftwareSerial::CONFIG_8N1; (and the rest)?

We'd wanna a patch release approach here for most identifiers, 3.2.0 come you can remove those if you want.

@dok-net
Copy link
Contributor Author

dok-net commented Mar 5, 2023

Why no SWSERIAL_8N1 = EspSoftwareSerial::CONFIG_8N1; (and the rest)?

I haven't explicitly tried now, but I suspect the parity would need the same. So I've reverted to SWSERIAL_8N1 etc. and added a using namespace EspSoftwareSerial. Unless now somebody's code has UART in conflict, it seems to be fine. I have undone most of the changes to the ESP8266 core's examples as well.

@dok-net dok-net changed the title EspSoftwareSerial 8.0.0: onReceive bug fix and new namespace EspSoftwareSerial 8.0.1: onReceive bug fix and new namespace Mar 10, 2023
@dok-net
Copy link
Contributor Author

dok-net commented Mar 10, 2023

I bumped up the bug fix release number just to make sure that Platformio doesn't keep using the wrong commit for 8.0.0 - it scans the main branch instead of looking at Github release tags. I like the Arduino library manager better :-)

@mcspr mcspr merged commit 2303632 into esp8266:master Mar 11, 2023
@mcspr mcspr added this to the 3.1.2 milestone Mar 11, 2023
@dok-net dok-net deleted the swserial branch March 11, 2023 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alpha included in alpha release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants