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

onError callback #329

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

onError callback #329

wants to merge 8 commits into from

Conversation

artemen
Copy link

@artemen artemen commented Jan 31, 2020

added a callback to notify user about a failed CRC packet.

src/LoRa.h Outdated
@@ -58,6 +58,7 @@ class LoRaClass : public Stream {
#ifndef ARDUINO_SAMD_MKRWAN1300
void onReceive(void(*callback)(int));
void onTxDone(void(*callback)());
void onError(void(*callback)());
Copy link
Owner

Choose a reason for hiding this comment

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

I'm thinking this should be renamed to something more specific, for example: onCrcError or onReceiveCrcError.

Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

Please name it in accordance with the rest of the repo. No objections.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @artemen if you can address these minor changes I'll merge this PR and include in the next release, which I'd like to get to this week.

Thanks for this contribution, it will add some important insight to error conditions.

@artemen
Copy link
Author

artemen commented Feb 24, 2020

sorry for the bit of mess. let me know if I should do a different pull request or this one will do, thanks!

@szerwi
Copy link

szerwi commented Aug 28, 2021

Do you plan to merge this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants