-
Notifications
You must be signed in to change notification settings - Fork 37
LoRa support #11
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
LoRa support #11
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately code which has been refactored already was un-refactored during this PR. Furthermore I request the code within LoRaConnectionHandler::update
to follow the same pattern as shown in WiFiConnectionHandler::update()
(the refactored version) in order to increase readability.
Hi @mirkokurt 👋 |
Hi @lxrobotics, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are getting closer but there still need to be some changes made 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are getting closer but there still need to be some changes made 😉
Hi @lxrobotics,
I've made the corrections and refactoring as you asked. I gave a short answer to each of the changes you've requested. Let me know if there are other changes I have to apply.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are getting in the right direction - however there have been many formatting changes which enlarges the change set that needs to be reviewed with not benefit. Please keep in mind to not do formatting changes when doing functional changes.
Also, if I mark formatting changes on some occasions I expect you to go through the whole change set and repair it everywhere it happens, not just where I remark on it.
Hi @lxrobotics, formatting changes have been applied automatically by my astyle code linter when I ran it. Usually I run the linter when I make a big refactoring to correctly format the code I just written. I didn't expect the linter to change the formatting of the code not written by me because I thought that the code was already in the astyle format. Now I restored the previous formatting. To avoid this in the future I propose to make a new standalone commit with only formatting changes to put all the files in full astyle format. |
@mirkokurt Since the PR seems to be complete now - have you tested it with all supported boards ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mirkokurt Since the PR seems to be complete now - have you tested it with all supported boards (
MKR GSM 1400
,MKR WiFi 1010
orMKR 1000
,MKR WAN
and a ESP8266 based board) that everything still works for all boards?
The LoRa support for Arduino IoTCloud implementation has a server counterpart. Tests are ongoing on both client and server side. Not every board in the list have been fully tested at the moment.
I've got slightly bad news for you. I was asked to merge the NB1500 connection handler PR into the master and have consequently released v0.2.0. Therefore this PR needs to be carefully rebased. |
|
It is alredy included in ConnectionHandler
the delay let the chip to be correctly initialized before the connection attempt
…aConnectionHandler
Done |
Support for LoRaWAN boards.
Connection handlers have been separated in TcpIP connection handlers and LPWAN connection handlers. For LPWAN protocols, a LoRa connection handler has been implemented. It uses the MKRWAN library: https://github.com/arduino-libraries/MKRWAN
To be used with:
arduino-libraries/ArduinoCloudThing#48
arduino-libraries/ArduinoIoTCloud#83