Skip to content

add NBConnectionHandler for mkrnb1500 #7

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

Merged
merged 1 commit into from
Dec 20, 2019

Conversation

AlbyIanna
Copy link
Contributor

No description provided.

@aentinger
Copy link
Contributor

@AlbyIanna Your PR looks pretty complete. Unfortunately I've got no board to test it myself, nonetheless I'd like to know if I can/shall merge this in?

@AlbyIanna
Copy link
Contributor Author

@lxrobotics There is only one thing that worries me: at the following line, I didn't know how to define NETWORK_HARDWARE_ERROR, because for the mkrgsm1400 we used GPRS_PING_ERROR, which I cannot use here because we don't do a ping to test the mkrnb1500 connection
https://github.com/arduino-libraries/Arduino_ConnectionHandler/pull/7/files#diff-5b06e1b92988019060e68def6ba17f7cR119

In fact, these lines don't makes much sense
https://github.com/arduino-libraries/Arduino_ConnectionHandler/pull/7/files#diff-18354ae7f5bbf67659d86e0d3fcabf68R123-R132

@aentinger
Copy link
Contributor

NETWORK_HARDWARE_ERROR need not to be defined, if there is no such error mode available. But from what I see the value of NETWORK_HARDWARE_ERROR could be NB_NetworkStatus_t::ERROR.

You are right, keeping the MKRGSM ping code does not make much sense. The main point at this place in code is to verify that there is actual a connection going on. Can you think of another possibility doing this "alive-"check?

@AlbyIanna
Copy link
Contributor Author

I have no idea 😕

@Rocketct
Copy link

Rocketct commented Nov 15, 2019

@lxrobotics as you say,the main problem is the fact that the SARA-R410 does not support the URC for the ping, in MKRNB library the access alive check is made checking the registration of the board to the network https://github.com/arduino-libraries/MKRNB/blob/master/src/NB.cpp#L109-L123

@aentinger
Copy link
Contributor

@Rocketct thank you very much for your input. It seems that NB::isAccessAlive could very much be used instead of the ping used for MKR GSM 1400.

@aentinger
Copy link
Contributor

Hey @AlbyIanna 👋
Since this PR is still a work-in-progress may I ask you to restructure NBConnectionHandler::update in a similiar way as I've restructed WiFiConnectionHandler::update? By extracting the state specific code in private member functions state transitions and errors can be seen much easier.
Thank you, Alex

@aentinger aentinger merged commit 8a981d5 into arduino-libraries:master Dec 20, 2019
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Jul 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants