Skip to content

Add overloaded constructors for optional apn, login and pass in NBConnectionHandler #12

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

Conversation

AlbyIanna
Copy link
Contributor

No description provided.

@AlbyIanna AlbyIanna force-pushed the AlbyIanna/NBConnectionHandler-optional-values branch from cec6740 to 0e1c597 Compare December 23, 2019 15:53
Copy link
Contributor

@sandeepmistry sandeepmistry left a comment

Choose a reason for hiding this comment

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

It's not clear the process for testing these changes and also I don't have a LTE Cat M1 SIM that requires a username + password to test these changes.

@@ -113,7 +123,7 @@ void NBConnectionHandler::update() {
case NetworkConnectionState::CONNECTING: {
// NOTE: Blocking Call when 4th parameter == true
NB_NetworkStatus_t networkStatus;
networkStatus = gprs.attachGPRS();
networkStatus = gprs.attachGPRS(apn, login, pass, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a default value for apn, login, or pass do they default to NULL or empty string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure, but I think they're NULL

Copy link
Contributor

Choose a reason for hiding this comment

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

I highly recommend they be explicitly initialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would you initialize those values? Can I do that in the signature?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I don’t think this compiles, here is the signature: https://github.com/arduino-libraries/MKRNB/blob/master/src/GPRS.h#L58

The parameters should be passed to ‘nbAccess.begin(...)’.

@AlbyIanna AlbyIanna force-pushed the AlbyIanna/NBConnectionHandler-optional-values branch from 0e1c597 to 8501341 Compare December 24, 2019 16:54
@facchinm facchinm merged commit 0708bf9 into arduino-libraries:master Dec 31, 2019
@AlbyIanna AlbyIanna deleted the AlbyIanna/NBConnectionHandler-optional-values branch January 7, 2020 08:36
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.

3 participants