Cellular: retry logic for CellularContext connect #10056

jarvte · 2019-03-12T12:23:40Z

Description

State machine has retry logic until device is attached to network.
After this CellularContext does the context activation e.g. connect.
There was no retry logic for context activation. Added logic to
CellularContext level so it's available for at and (upcoming) ril layers.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@AriParkkila

Release Notes

Add retry logic for CellularContext connect last phase: activating pdp context / open ppp channel.
Retry logic is the same as in CellularStateMachine.

ciarmcom · 2019-03-12T14:00:42Z

@jarvte, thank you for your changes.
@AriParkkila @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-wan @ARMmbed/mbed-os-maintainers please review.

0xc0170 · 2019-03-13T10:16:16Z

Added retry logic for CellularContext connect last phase: activating pdp context / open ppp channel.

Is this still considered a fix rather than adding new functionality (new method to cellularcontext) as result it will fix as well.

AnttiKauppila · 2019-03-13T10:34:27Z

features/cellular/framework/API/CellularContext.h

+     */
+    void call_network_cb(nsapi_connection_status_t status);
+
+    virtual void do_connect() = 0;


This is an API break!

These are protected methods and already implemented in AT_CellularContext class (exepct do_connect_with_retry). This was done so that RIL and AT layers don't have copy-paste code. So is this kind of change also prohibited?

Pure virtual will cause code changes, while a function with a default implementation will not.

Ok, if I change this to not pure virtual but virtual it's accepted?

Agreed fix to virtual method with default implementation

pushed fix. Now there is a default implementation in place.

AnttiKauppila

Why a new API break introduced?

jarvte · 2019-03-13T11:03:43Z

Why a new API break introduced?

see my response to your other comment

SeppoTakalo

These are internal APIs.
I have no objection from IP Core point of view. Needs Cellular tech-lead to approve.

SeppoTakalo · 2019-03-13T13:32:36Z

features/cellular/framework/AT/AT_CellularContext.cpp

@@ -106,6 +112,11 @@ void AT_CellularContext::enable_hup(bool enable)
    }
 }

+void AT_CellularContext::do_connect_with_retry()
+{
+    CellularContext::do_connect_with_retry();


Why do you override if you just directly call the base?

It's because of EventQueue. In non blocking mode we use EventQueue to call do_connect_with_retry. We can't call CellularContext::do_connect_with_retry() as it's protected so to avoid that problem we had to introduce this function.

jarvte · 2019-03-14T06:13:46Z

Added retry logic for CellularContext connect last phase: activating pdp context / open ppp channel.

Is this still considered a fix rather than adding new functionality (new method to cellularcontext) as result it will fix as well.

New method is a protected method but I'm fine with fix or functionality change. I really don't know which would suite better.

jarvte · 2019-03-14T08:24:03Z

Force pushed doxygen updates.

jarvte · 2019-03-27T06:13:07Z

ping

cmonr · 2019-03-27T19:30:27Z

@ARMmbed/mbed-os-wan Please re-review.

jarvte · 2019-03-29T06:04:09Z

Force pushed rebase with master to fix conflicts.

State machine has retry logic until device is attached to network. After this CellularContext does the context activation e.g. connect. There was no retry logic for context activation. Added logic to CellularContext level so it's available for at and (upcoming)ril layers.

cmonr · 2019-03-29T19:12:27Z

Info: This PR has now been rebased.

If this was made in error, feel free to force-push your local branch to restore the PR.

cmonr · 2019-03-29T19:12:44Z

CI started

cmonr · 2019-03-29T19:13:19Z

@jarvte Rebased your PR since a seperate PR fixed the astyle issues.

mbed-ci · 2019-03-29T23:48:00Z

Test run: FAILED

Summary: 2 of 13 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test
jenkins-ci/mbed-os-ci_exporter

NirSonnenschein · 2019-03-31T08:57:46Z

Failures seem to be CI K66 issue +license issue. restarting CI

mbed-ci · 2019-03-31T12:18:15Z

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

jarvte · 2019-04-01T05:41:26Z

Still K66F failing, not related to this pr.

alekla01 · 2019-04-01T07:50:09Z

restarted jenkins-ci/mbed-os-ci_greentea-test

mbed-ci · 2019-04-01T13:25:47Z

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 3
Build artifacts

0xc0170 · 2019-04-05T12:10:45Z

@jarvte Need help with this fix. Patching this causing multiple conflicts that are not that easy to resolve - too many changes on master landing to these files? Need a branch against release-candidate branch (already having 5.12.1rc ready, just waiting for this one to land).

0xc0170 · 2019-04-05T12:12:25Z

Or will go to 5.12.2 with new PR targeting 5.12 branch

0xc0170 · 2019-04-05T12:46:18Z

Moving to 5.12.2 (if wont be manually patched for 5.12.1).

cmonr · 2019-04-09T16:47:29Z

Moving to 5.13.0-rc1.

ciarmcom requested review from AriParkkila and a team March 12, 2019 14:00

ciarmcom added the needs: review label Mar 12, 2019

AriParkkila approved these changes Mar 13, 2019

View reviewed changes

AnttiKauppila reviewed Mar 13, 2019

View reviewed changes

AnttiKauppila suggested changes Mar 13, 2019

View reviewed changes

SeppoTakalo approved these changes Mar 13, 2019

View reviewed changes

jarvte force-pushed the context_act_ppp_retry_logic branch from 00b85ef to 5626df9 Compare March 13, 2019 14:34

AnttiKauppila approved these changes Mar 14, 2019

View reviewed changes

jarvte force-pushed the context_act_ppp_retry_logic branch from 5626df9 to 8e58962 Compare March 14, 2019 08:22

cmonr added the release-version: 5.12.1 label Mar 15, 2019

cmonr approved these changes Mar 27, 2019

View reviewed changes

jarvte force-pushed the context_act_ppp_retry_logic branch from 8e58962 to b4b7128 Compare March 29, 2019 06:01

AriParkkila approved these changes Mar 29, 2019

View reviewed changes

cmonr force-pushed the context_act_ppp_retry_logic branch from b4b7128 to c6e5595 Compare March 29, 2019 19:12

cmonr added the needs: CI label Mar 29, 2019

NirSonnenschein removed the needs: review label Mar 31, 2019

0xc0170 added ready for merge and removed needs: CI labels Apr 1, 2019

cmonr merged commit cf4118f into ARMmbed:master Apr 1, 2019

cmonr removed the ready for merge label Apr 1, 2019

0xc0170 added Manually Patch and removed release-version: 5.12.1 labels Apr 5, 2019

jarvte deleted the context_act_ppp_retry_logic branch April 5, 2019 10:20

0xc0170 mentioned this pull request Apr 5, 2019

Release candidate for mbed-os-5.12.1 #10326

Merged

0xc0170 added release-version: 5.12.2 and removed Manually Patch labels Apr 5, 2019

cmonr added release-version: 5.13.0-rc1 and removed release-version: 5.12.2 labels Apr 9, 2019

Cellular: retry logic for CellularContext connect #10056

Cellular: retry logic for CellularContext connect #10056

Uh oh!

Conversation

jarvte commented Mar 12, 2019 • edited by 0xc0170 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Release Notes

Uh oh!

ciarmcom commented Mar 12, 2019

Uh oh!

0xc0170 commented Mar 13, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AnttiKauppila left a comment

Choose a reason for hiding this comment

Uh oh!

jarvte commented Mar 13, 2019

Uh oh!

SeppoTakalo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jarvte commented Mar 14, 2019

Uh oh!

jarvte commented Mar 14, 2019

Uh oh!

jarvte commented Mar 27, 2019

Uh oh!

cmonr commented Mar 27, 2019

Uh oh!

jarvte commented Mar 29, 2019

Uh oh!

cmonr commented Mar 29, 2019

Uh oh!

cmonr commented Mar 29, 2019

Uh oh!

cmonr commented Mar 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbed-ci commented Mar 29, 2019

Uh oh!

NirSonnenschein commented Mar 31, 2019

Uh oh!

mbed-ci commented Mar 31, 2019

Uh oh!

jarvte commented Apr 1, 2019

Uh oh!

alekla01 commented Apr 1, 2019

Uh oh!

mbed-ci commented Apr 1, 2019

Uh oh!

0xc0170 commented Apr 5, 2019

Uh oh!

0xc0170 commented Apr 5, 2019

Uh oh!

0xc0170 commented Apr 5, 2019

Uh oh!

cmonr commented Apr 9, 2019

Uh oh!

Uh oh!

jarvte commented Mar 12, 2019 •

edited by 0xc0170

Loading

cmonr commented Mar 29, 2019 •

edited

Loading