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

add NO_SW_SERIAL_REQUIRED define for software serial compatibility #24

Merged
merged 3 commits into from
May 16, 2022

Conversation

pintert3
Copy link
Contributor

I was having a problem with using this library alongside those that use software serial. If I deliberately wanted to avoid using Software serial in order to avoid clashes with other libraries like the Arduino-SDI-12, but needed this, I couldn't previously. This was my solution which worked.

It's 5months old though, I forgot to offer it earlier. But it's probably still relevant given that I see no new commits of recent.

@pintert3
Copy link
Contributor Author

Oh btw, this would mean making changes to the example code. I'm guessing the SoftwareSerial code would expecially suffer since it would expect it to already be imported. Could add that too.

@pintert3
Copy link
Contributor Author

Actually, on second thought, I might close this as I see it's a pretty minor problem with many ways to solve it. The HAS_SW_SERIAL should be enough of a hint to anyone having it in my opinion. I'll leave it for some days to see if any useful comments show up and then close.

@avaldebe
Copy link
Owner

Hi @pintert3

Thanks for the PR.
The CI failed, but it seems the be unrelated.

I need to fix the CI before going forward.
In the meantime, I would like you to address the the following points:

As fare as I can tell SW_REQUIRED is undefined by default.
This changes the default behaviour of the library.
In order to keep the default behaviour of the library,
the compiling directive should be some thing like:

#if (defined(__AVR__) || defined(ESP8266)) && !defined(NO_SW_REQUIRED)
#define HAS_SW_SERIAL
#include <SoftwareSerial.h>
#endif

Also, could you add a line or two about it to this option README.md?
A code snippet showing how to load the library would be just the thing.

@pintert3
Copy link
Contributor Author

Sorry for the delayed response, been quite busy in the last few months. I've changed the behaviour to NO_SW_SERIAL_REQUIRED and added a note in the readme.

@pintert3 pintert3 changed the title add SW_REQUIRED define for software serial compatibility add NO_SW_SERIAL_REQUIRED define for software serial compatibility May 13, 2022
Copy link
Owner

@avaldebe avaldebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for following up on this.
I need to move this project from Travis CI to GH actions before merging this PR.
Sorry for the delay...

@avaldebe avaldebe merged commit 5126ea9 into avaldebe:master May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants