Cellular: Update cellular debug prints #8767

AriParkkila · 2018-11-16T07:33:41Z

Description

Cellular traces need to be improved:

Removed unnecessary prints.
Tracing more in DEBUG level.
Read/write bytes not printed on big packets.
Signal quality (RSSI) traced to log network problems.
Dismissed AT data is traced.
Modem type and firmware version are traced.

Network registration returns NotRegistered instead of previous StatusNotAvailable (which is not part of 3GPP TS 27.007).

Pull request type

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

AriParkkila · 2018-11-16T07:34:13Z

@mirelachirica @jarvte please review.

features/cellular/framework/AT/AT_CellularDevice.cpp

features/cellular/framework/device/CellularStateMachine.cpp

0xc0170 · 2018-11-16T09:47:41Z

I recommend rebasing now ( to get latest astyle update) and get the latest numbers for styling changes

AriParkkila · 2018-11-16T10:14:00Z

@jarvte fixed debug ifdefs. I'd like to keep timeouts, there are cases when we may need to wait up to 3 minutes without anything happening on modem.

@0xc0170 yes, rebase helped to get rid of excessive coding guideline fixes.

0xc0170

Can you add the texts from description to the commit msg as well?

UNITTESTS/features/cellular/framework/AT/at_cellularnetwork/at_cellularnetworktest.cpp

features/cellular/framework/API/CellularNetwork.h

AriParkkila · 2018-11-16T11:12:01Z

@0xc0170 enum was added just to mark how many debug string need to be in array.

Unittests expected value that can't be printed because value was not part of AT spec, but yes, strictly speaking that is a change that I can revert.

0xc0170 · 2018-11-16T11:15:28Z

Unittests expected value that can't be printed because value was not part of AT spec, but yes, strictly speaking that is a change that I can revert.

Can leave it but separate it with decription. Not certain why it was squashed all to one commit with one-line commit msg

0xc0170 · 2018-11-19T10:31:41Z

@AriParkkila Is this aiming to get into 5.11 ? If yes, please let us know and adress the latest comments here

Updated cellular debug trace prints: - Removed unnecessary prints. - Tracing more in DEBUG level. - Read/write bytes not printed on big packets. - Signal quality (RSSI) traced to log network problems. - Dismissed AT data is traced. - Modem type and firmware version are traced. Network registration returns NotRegistered instead of StatusNotAvailable, because that's not in 3GPP TS 27.007.

AriParkkila · 2018-11-19T10:59:18Z

@0xc0170 updated commit message, so I guess there are no more open requests.

mbed-ci · 2018-11-19T11:54:04Z

Test run: FAILED

Summary: 4 of 7 test jobs failed
Build number : 2
Build artifacts
Build logs

Failed test jobs:

jenkins-ci/mbed-os-ci_mbed2-build-ARM
jenkins-ci/mbed-os-ci_mbed2-build-IAR
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_mbed2-build-GCC_ARM

0xc0170 · 2018-11-19T15:32:07Z

Triggering the build + exporters (test might be aborted due to some issues there)

/morph build

mbed-ci · 2018-11-19T16:40:11Z

Build : SUCCESS

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

Triggering tests

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

mbed-ci · 2018-11-19T18:28:12Z

Exporter Build : SUCCESS

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

0xc0170 · 2018-11-22T12:58:43Z

Info: This PR has been re-bundled into a new rollup PR (#8838 ).

No further work is needed here, as once that PR is merged, this PR will also be closed and marked as merged.
If any more commits are made in this PR, this PR will remain open and have to go through CI on its own.

0xc0170 · 2018-11-24T20:38:42Z

features/cellular/framework/AT/AT_CellularNetwork.cpp

@@ -485,6 +496,12 @@ nsapi_error_t AT_CellularNetwork::get_signal_quality(int &rssi, int &ber)
        return NSAPI_ERROR_DEVICE_ERROR;
    }

+    if (rssi == 99) {


Although this was already integrated, during the rollup review, these numbers were questions - I assume rssi is a value 0-99, but then what below means, on line 502 might help to add a comment

@0xc0170 RSSI value is always in dBm, and actual range is -51 ... -113 dBm Previously returned values 0..99 were raw values from AT protocol spec due to conversion implementation was missing.

That explains it, however we have in the code magic values that do not , either create defines or add a comment describing this calculation. You can send a new PR

@0xc0170 thx, I will fix that within another PR.

jarvte suggested changes Nov 16, 2018

View reviewed changes

features/cellular/framework/AT/AT_CellularDevice.cpp Show resolved Hide resolved

features/cellular/framework/device/CellularStateMachine.cpp Show resolved Hide resolved

features/cellular/framework/device/CellularStateMachine.cpp Show resolved Hide resolved

0xc0170 added the needs: work label Nov 16, 2018

AriParkkila force-pushed the cell-trace branch from 3665457 to fb14d6a Compare November 16, 2018 09:07

AriParkkila force-pushed the cell-trace branch from fb14d6a to edd183f Compare November 16, 2018 10:08

AriParkkila changed the title ~~Cellular: Update coding style and debug prints~~ Cellular: Updated cellular debug prints Nov 16, 2018

jarvte approved these changes Nov 16, 2018

View reviewed changes

0xc0170 added needs: review and removed needs: work labels Nov 16, 2018

0xc0170 requested a review from a team November 16, 2018 11:03

0xc0170 requested changes Nov 16, 2018

View reviewed changes

UNITTESTS/features/cellular/framework/AT/at_cellularnetwork/at_cellularnetworktest.cpp Show resolved Hide resolved

features/cellular/framework/API/CellularNetwork.h Show resolved Hide resolved

cmonr added needs: work and removed needs: review labels Nov 16, 2018

0xc0170 changed the title ~~Cellular: Updated cellular debug prints~~ Cellular: Update cellular debug prints Nov 19, 2018

AriParkkila force-pushed the cell-trace branch from edd183f to 9bdeb68 Compare November 19, 2018 10:56

0xc0170 approved these changes Nov 19, 2018

View reviewed changes

0xc0170 added needs: CI and removed needs: work labels Nov 19, 2018

0xc0170 requested a review from mirelachirica November 19, 2018 13:02

0xc0170 added the rollup PR label Nov 22, 2018

0xc0170 mentioned this pull request Nov 22, 2018

Rollup v2 for 5.11rc1 #8838

Merged

0xc0170 added the release-version: 5.11.0-rc1 label Nov 24, 2018

0xc0170 merged commit 9bdeb68 into ARMmbed:master Nov 24, 2018

0xc0170 removed the needs: CI label Nov 24, 2018

0xc0170 reviewed Nov 24, 2018

View reviewed changes

cmonr removed the rollup PR label Nov 26, 2018

Cellular: Update cellular debug prints #8767

Cellular: Update cellular debug prints #8767

Uh oh!

Conversation

AriParkkila commented Nov 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Uh oh!

AriParkkila commented Nov 16, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

0xc0170 commented Nov 16, 2018

Uh oh!

AriParkkila commented Nov 16, 2018

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AriParkkila commented Nov 16, 2018

Uh oh!

0xc0170 commented Nov 16, 2018

Uh oh!

0xc0170 commented Nov 19, 2018

Uh oh!

AriParkkila commented Nov 19, 2018

Uh oh!

mbed-ci commented Nov 19, 2018

Test run: FAILED

Uh oh!

0xc0170 commented Nov 19, 2018

Uh oh!

mbed-ci commented Nov 19, 2018

Build : SUCCESS

Triggering tests

Uh oh!

mbed-ci commented Nov 19, 2018

Exporter Build : SUCCESS

Uh oh!

0xc0170 commented Nov 22, 2018

Uh oh!

0xc0170 Nov 24, 2018

Choose a reason for hiding this comment

Uh oh!

AriParkkila Nov 26, 2018

Choose a reason for hiding this comment

Uh oh!

0xc0170 Nov 26, 2018

Choose a reason for hiding this comment

Uh oh!

AriParkkila Nov 26, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AriParkkila commented Nov 16, 2018 •

edited

Loading