RN2483-Arduino-Library
RN2483-Arduino-Library copied to clipboard
jpmeijers
Make handling commands non-blocking via optional async mode
This rewrite does make the handling of the commands non-blocking, so you can use it along with other tasks on the node.
The interface of the library is kept the same as much as possible.
This is quite a big (huge) rewrite, so you may also consider it is not really fitting as a pull request for your library. If so, then I will make it a separate repository.
Apparently Travis failed over the (not yet updated) examples. I still have to fix those examples to be compatible again with the changes.
Wow that is quite an impressive change. Thanks for the contribution!
@jwillemsen what do you think? Should we merge this in?
I'm of the opinion that it can be merged in as long as backwards compatibility is kept (or with minor changes). Also the other Arduino RN2xx3 library - The Things Network library - does things in a synchronous manner, so being asynchronous gives this library a new meaning to exist.
I am running it in my ESPEasy project as a controller, so it does need to do a lot of other stuff in the background on an ESP8266.
Handling the I/O is now quite fast, as most commands can be executed in 20 - 50 msec (@57600 baud) and sending data via mac tx does not take longer than 80 msec to make sure the data is accepted by the module.
Well that's with async mode enabled, which is something you have to enable by calling a function to switch on that mode.
If you don't enable it, it will work as before, although it will be quite a bit more responsive if no module is connected or not configured at the defined pins.
The used timeouts are:
- 1500 msec for any command's first reply
- rxdelay2 + 3000 msec for
mac txreplies in the rx2 window. - 10 seconds for OTAA join (after the first reply)
Lot of changes, haven't look at in detail but there is no new example that shows how to use the async feature, without that it is hard for anyone other to use. That example should also be compiled on travis
Yes. A library lives or dies not on the quality of its code, but on the quality of its documentation.
On Mon, 17 Feb 2020, 07:38 Johnny Willemsen, [email protected] wrote:
Lot of changes, haven't look at in detail but there is no new example that shows how to use the async feature, without that it is hard for anyone other to use. That example should also be compiled on travis
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jpmeijers/RN2483-Arduino-Library/pull/75?email_source=notifications&email_token=AFAGDCV5BIMBVDN3PYFF5HDRDI5GTA5CNFSM4KWFPLPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEL5LJ2I#issuecomment-586855657, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFAGDCX6GIERGL7VMTWHOX3RDI5GTANCNFSM4KWFPLPA .
Sure I will add an example to use the async mode. It was first an initial pull request pending a reply to see if you even would like to consider merging it. But if you like that, I will add a few examples.
I just added a simple example to illustrate the async mode, but have still to test it myself. I am not near a node with LoRa right now, but got some time to make a quick example.
Maybe you can also test it yourself, and I will later test it myself too and let you know if it is working :)
N.B. I guess there should also be something written about this in documentation files?
Just my two cents, found this library a few days ago and want to adapt for a current project not on Arduino, this PR almost totally rewrites the whole library IMHO. The current master state implements just some convenience methods and shortcuts for simple interaction with the module, bundling certain functions and processes into one call. Except some details it is not really just usable for Arduino. This PR changes that, so I think this should be published as a separated library...
This PR changes that, so I think this should be published as a separated library...
That's also fine with me. The PR as it is, is now in use on my nodes since Februari, when I created this PR and so far I only found one issue where the module can remain trapped in an "eternal busy state" which requires a reset of the module. That does seem to be a bug in the module itself, but it would be nice to have it at least detected in the library so the state can be set to "requires reset". I do have code for it running on one node, but so far the module just behaved nicely :(
@jpmeijers Please let me know what you think is best to do, merge this or consider it a forked library?
It seems like after merging your first PR https://github.com/jpmeijers/RN2483-Arduino-Library/pull/66 this one has conflicts and can't be merged.
It seems like after merging your first PR #66 this one has conflicts and can't be merged.
I will have a look at it.
OK, was working for 40-ish minutes on the merge, then another commit was merged so it would not allow me to commit my merge. :(
Will take another look later today (or tomorrow)
Sorry for that. I was trying to clean up old trivial PRs which only changes single lines and should not break merges.
Funny though, because this PR is pointing to a dedicated branch which should not have changed.
No rush, I'm the one that is years behind on admin on this project. Thanks a lot for all the contributions you made over the years.
Merges via GitHub (not local) will no longer be accepted when the main branch changed. So then I lost the possibility to commit them.
I will later try to merge it locally, as that does give more insights in what was changed in the main branch.