Skip to content

Cellular: Fixed con-disc sequence can now be called multiple times. #6928

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 29, 2018
Merged

Cellular: Fixed con-disc sequence can now be called multiple times. #6928

merged 3 commits into from
May 29, 2018

Conversation

jarvte
Copy link
Contributor

@jarvte jarvte commented May 16, 2018

Description

Fixed connect-disconnect sequence so that it can now be called multiple times from EasyCellularConnection.
Minor improvements to cellular state machine to make connect faster.

Internal ref to defect: IOTCELL-995

@AriParkkila @mirelachirica please review

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

AriParkkila
AriParkkila approved these changes May 17, 2018
View reviewed changes
features/cellular/easy_cellular/EasyCellularConnection.cpp Outdated
}
}

void EasyCellularConnection::set_plmn(const char* plmn)
{
_cellularConnectionFSM.set_plmn(plmn);
if (_cellularConnectionFSM) {
Copy link

Choose a reason for hiding this comment

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

should we take plmn in temporary store or init FSM similar to set_sim_pin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, will add init

features/cellular/easy_cellular/EasyCellularConnection.cpp
_credentials_err = init();

if (_credentials_err) {
return;
Copy link

Choose a reason for hiding this comment

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

should we just assert here..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_credentials_err is used later when trying to connect to return proper error. Because of the inherited void function we must store error for later use.

AriParkkila
AriParkkila previously approved these changes May 17, 2018
View reviewed changes
@jarvte
Copy link
Contributor Author

jarvte commented May 21, 2018

Please proceed with this one.

0xc0170
0xc0170 previously approved these changes May 22, 2018
View reviewed changes
@0xc0170
Copy link
Contributor

0xc0170 commented May 22, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented May 22, 2018

Build : FAILURE

Build number : 2100
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6928/

@jarvte
Copy link
Contributor Author

jarvte commented May 22, 2018

@0xc0170 morph pc out of space?

@0xc0170
Copy link
Contributor

0xc0170 commented May 22, 2018 •
edited
Loading

@0xc0170 morph pc out of space?

Looks like that. I already noticed this a moment ago and reported. We will restart it

@cmonr
Copy link
Contributor

cmonr commented May 23, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented May 23, 2018

Build : FAILURE

Build number : 2121
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6928/

0xc0170
0xc0170 reviewed May 23, 2018
View reviewed changes
features/cellular/easy_cellular/CellularConnectionFSM.cpp Outdated
}
}
if (!success) {
tr_warning("Failed to set any URC's for registration");
Copy link
Contributor

Choose a reason for hiding this comment

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

Please review the build logs. Looks like this is not defined (missing include ?) .

@jarvte jarvte dismissed stale reviews from 0xc0170 and AriParkkila via 03f50e6 May 23, 2018 19:02
cmonr
cmonr approved these changes May 23, 2018
View reviewed changes
Copy link
Contributor

@cmonr cmonr left a comment

Choose a reason for hiding this comment

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

Fix LGTM

@cmonr
Copy link
Contributor

cmonr commented May 23, 2018

Holding off on restarting CI since we're manually prioritizing builds for PRs going into master in prep for 5.9 code freeze.

Since this is a fix, it can come in during 5.8.6.

@0xc0170
Copy link
Contributor

0xc0170 commented May 26, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented May 26, 2018

Build : SUCCESS

Build number : 2162
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6928/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented May 26, 2018

Test : SUCCESS

Build number : 1952
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/6928/1952

@mbed-ci
Copy link

mbed-ci commented May 26, 2018

Exporter Build : SUCCESS

Build number : 1788
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/6928/

@cmonr cmonr merged commit 917fe08 into ARMmbed:master May 29, 2018
@jarvte jarvte deleted the fix_cellular_connect_disconnect branch June 4, 2018 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 left review comments

@cmonr cmonr cmonr approved these changes

@AriParkkila AriParkkila AriParkkila left review comments

@mirelachirica mirelachirica Awaiting requested review from mirelachirica

Assignees
No one assigned
Labels
release-version: 5.9.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jarvte @0xc0170 @mbed-ci @cmonr @AriParkkila