LoRaWAN: Style and bug fixes #6938

hasnainvirk · 2018-05-17T09:13:34Z

Description

This PR fixes a few bugs and does most important style corrections as pointed out by CI.
TARGET: 5.9

Pull request type

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

hasnainvirk · 2018-05-17T09:14:06Z

@0xc0170 @kivaisan @kjbracey-arm Please review.

0xc0170 · 2018-05-17T11:19:42Z

@hasnainvirk 👍 , I can still see issues in AStyle, not certain if those files are ours or not (if 3rd party, we shall add them to astyle ignore). Please review

hasnainvirk · 2018-05-17T12:22:36Z

@0xc0170 Yes there are still warnings. I did not change them as that bloats the code. It becomes unreadable if I follow the style guide.

kjbracey · 2018-05-17T12:36:58Z

Couple of astyle oddities there:

why is it sometimes pulling multiline function arguments to the left and sometimes aligning with the first argument? Can't see a line length reason. It doesn't seem to have a consistent rule.
It's sticking an extra level of indent in for cases incorporating curly brackets, making 3(!) levels of indent. Any way to avoid that?

I wouldn't generally take any tool too seriously on multiline alignment, but it's right on lots of the other stuff, like spaces around brackets and asterisks.

0xc0170 · 2018-05-17T13:00:08Z

Thanks @kjbracey-arm , will look at those oddities and report back. Not a blocker for this PR, but

I wouldn't generally take any tool too seriously on multiline alignment, but it's right on lots of the other stuff, like spaces around brackets and asterisks.

At least this could be fixed once for all.

hasnainvirk · 2018-05-21T12:14:00Z

Please review. @kjbracey-arm @kivaisan @0xc0170

kivaisan · 2018-05-21T12:16:13Z

features/lorawan/lorastack/mac/LoRaMac.cpp

+                (params->connection_u.abp.nwk_id == 0)      ||
+                (params->connection_u.abp.nwk_skey == NULL) ||
+                (params->connection_u.abp.app_skey == NULL)) {
+                return LORAWAN_STATUS_PARAMETER_INVALID;


Change these to the same style as aboce done for OTAA checks. (|| operator at the beginning of the line.)

hasnainvirk · 2018-05-21T13:25:24Z

@kjbracey-arm Please review again.

hasnainvirk · 2018-05-21T13:26:19Z

@0xc0170 Same CI failure here as well. Please kick start the job again.

hasnainvirk · 2018-05-22T11:57:16Z

@cmonr This needs CI. Actually this and #6960 can potentially go in before #6910

cmonr · 2018-05-22T19:34:44Z

We'll start CI as soon as we can. Working through an issue behind the scenes.

0xc0170 · 2018-05-24T09:57:30Z

@hasnainvirk Can you rebase?

By the way , I am reviewing those astyle issues . I believe I already had it fixed but then reworked entire branch and this got lost. Will share a branch with a fix, that should make all lorawan code look good hopefully.

hasnainvirk · 2018-05-24T13:55:10Z

@0xc0170 Rebased. Please review and CI required.

hasnainvirk · 2018-05-25T07:51:21Z

@0xc0170 Could you please investigate why it failed ?

0xc0170 · 2018-05-26T10:58:13Z

Needs a rebase after another patch made it in. we can then start CI

cmonr · 2018-05-31T15:28:00Z

@hasnainvirk Looks like another rebase will be needed.

A few of the structures were missing from the storage space which is visible topublic APIs. Suc structures are now being added.

Travis astyle check pointed out some of the style mismatches in the code. Not all of them are worth changing as they make the code unreadable and some of them are semantically wrong. So in this commit, we have attempted to pick the most important style mismatches and rectify.

If the value is an integer, the 4th bit is used for sign, so you can store values upto 7 only whereas the datarate values could go upto 15. That's why we need to turn this to an unsigned integer so that the last bit can also be used.

Check that ABP dev_addr and nwk_id is non-zero and keys are not null.

cmonr · 2018-06-04T16:06:35Z

@kjbracey-arm @kivaisan Mind doing a re-review?

cmonr · 2018-06-06T16:19:11Z

/morph build

mbed-ci · 2018-06-06T22:41:34Z

Build : SUCCESS

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

Triggering tests

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

mbed-ci · 2018-06-07T05:44:10Z

Test : SUCCESS

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

mbed-ci · 2018-06-07T10:12:44Z

Exporter Build : SUCCESS

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

hasnainvirk · 2018-06-11T08:12:31Z

@cmonr @0xc0170 Can this go in please ?

hasnainvirk force-pushed the style_and_bug_fixes branch from 9b056df to 93fa698 Compare May 17, 2018 09:18

kivaisan approved these changes May 17, 2018

View reviewed changes

hasnainvirk force-pushed the style_and_bug_fixes branch from 93fa698 to 1baf2a4 Compare May 17, 2018 09:39

0xc0170 added the needs: review label May 17, 2018

hasnainvirk force-pushed the style_and_bug_fixes branch 3 times, most recently from 4925543 to 6764e00 Compare May 21, 2018 12:12

kjbracey previously approved these changes May 21, 2018

View reviewed changes

kivaisan previously approved these changes May 21, 2018

View reviewed changes

hasnainvirk dismissed kjbracey’s stale review via ea19998 May 21, 2018 12:19

hasnainvirk force-pushed the style_and_bug_fixes branch from 6764e00 to ea19998 Compare May 21, 2018 12:19

kjbracey previously approved these changes May 21, 2018

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels May 24, 2018

hasnainvirk dismissed stale reviews from kjbracey and kivaisan via 69f23a0 May 24, 2018 13:54

hasnainvirk force-pushed the style_and_bug_fixes branch from ea19998 to 69f23a0 Compare May 24, 2018 13:54

0xc0170 added needs: work and removed needs: CI labels May 26, 2018

cmonr added the release-version: 5.9.1 label May 31, 2018

Kimmo Vaisanen and others added 5 commits June 1, 2018 12:29

Lora: All API methods should check if stack has been initialized

d3327c3

Moving missing public data structures

e438846

A few of the structures were missing from the storage space which is visible topublic APIs. Suc structures are now being added.

dr_range bitfield should be unsigned integer

493ae2b

If the value is an integer, the 4th bit is used for sign, so you can store values upto 7 only whereas the datarate values could go upto 15. That's why we need to turn this to an unsigned integer so that the last bit can also be used.

Lora: Check for valid ABP params

0f0b3be

Check that ABP dev_addr and nwk_id is non-zero and keys are not null.

hasnainvirk dismissed kjbracey’s stale review via 0f0b3be June 1, 2018 09:41

hasnainvirk force-pushed the style_and_bug_fixes branch from 69f23a0 to 0f0b3be Compare June 1, 2018 09:41

cmonr added needs: review and removed needs: work labels Jun 4, 2018

kivaisan approved these changes Jun 5, 2018

View reviewed changes

cmonr added needs: CI and removed needs: review labels Jun 5, 2018

cmonr added needs: review and removed needs: CI labels Jun 7, 2018

kjbracey approved these changes Jun 8, 2018

View reviewed changes

kjbracey added ready for merge and removed needs: review labels Jun 11, 2018

cmonr merged commit b00a91d into ARMmbed:master Jun 11, 2018

0xc0170 removed the ready for merge label Jun 11, 2018

LoRaWAN: Style and bug fixes #6938

LoRaWAN: Style and bug fixes #6938

Uh oh!

Conversation

hasnainvirk commented May 17, 2018

Description

Pull request type

Uh oh!

hasnainvirk commented May 17, 2018

Uh oh!

0xc0170 commented May 17, 2018

Uh oh!

hasnainvirk commented May 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kjbracey commented May 17, 2018

Uh oh!

0xc0170 commented May 17, 2018

Uh oh!

hasnainvirk commented May 21, 2018

Uh oh!

kivaisan May 21, 2018

Choose a reason for hiding this comment

Uh oh!

hasnainvirk commented May 21, 2018

Uh oh!

hasnainvirk commented May 21, 2018

Uh oh!

hasnainvirk commented May 22, 2018

Uh oh!

cmonr commented May 22, 2018

Uh oh!

0xc0170 commented May 24, 2018

Uh oh!

hasnainvirk commented May 24, 2018

Uh oh!

hasnainvirk commented May 25, 2018

Uh oh!

0xc0170 commented May 26, 2018

Uh oh!

cmonr commented May 31, 2018

Uh oh!

cmonr commented Jun 4, 2018

Uh oh!

cmonr commented Jun 6, 2018

Uh oh!

mbed-ci commented Jun 6, 2018

Build : SUCCESS

Triggering tests

Uh oh!

mbed-ci commented Jun 7, 2018

Test : SUCCESS

Uh oh!

mbed-ci commented Jun 7, 2018

Exporter Build : SUCCESS

Uh oh!

hasnainvirk commented Jun 11, 2018

Uh oh!

Uh oh!

hasnainvirk commented May 17, 2018 •

edited

Loading