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

LoRaWAN: Message flags correction #6960

Merged
merged 1 commit into from May 24, 2018

Conversation

Projects
None yet
7 participants
@hasnainvirk
Contributor

hasnainvirk commented May 21, 2018

Description

Uplink multicast is not allowed. Proprietary messages cannot be
of type unconfirmed and unconfirmed.

Some error flags are introduced which check in the UP direction
if the user have used the flags wrongly.

Target version

Mbed OS-5.9-rc1

Pull request type

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

This comment has been minimized.

Contributor

hasnainvirk commented May 21, 2018

@hasnainvirk hasnainvirk changed the title from Message flags correction to LoRaWAN: Message flags correction May 21, 2018

@hasnainvirk

This comment has been minimized.

Contributor

hasnainvirk commented May 21, 2018

@0xc0170 Could you please reset the CI build for this PR ?

}
// case 2: check if both CONFIRMED and UNCONFIRMED flags set
if ((flags & ERR_FLAG_MUTUAL_EXCLUSION) == ERR_FLAG_MUTUAL_EXCLUSION) {

This comment has been minimized.

@kjbracey-arm

kjbracey-arm May 21, 2018

Contributor

What if neither are set (and it's a normal message)? Seems like this might be simpler if confirmed/unconfirmed was a single flag.

This is all very wordy to do a very simple test. I'd be vaguely inclined to just list out the valid values thus:

switch (flags) {   /* or (flags & MSG_FLAG_MASK) ? */
     case UNCONFIRMED:
     case CONFIRMED:
     case PROPRIETARY:
          break;
  
     default:
          tr_error("Invalid send flags");
          return LORAWAN_STATUS_PARAMETER_INVALID;
 }

This comment has been minimized.

@hasnainvirk

hasnainvirk May 21, 2018

Contributor

case 1 handles if nothing is set. Multicast flag cannot be set for uplink. And all the flags are mutually exclusive, so they can't be used in pairs for uplink.

Message flags correction
Uplink multicast is not allowed. Proprietary messages cannot be
of type unconfirmed and unconfirmed.

@hasnainvirk hasnainvirk force-pushed the hasnainvirk:msg_flags branch from d1a9bfd to 71348f7 May 21, 2018

@hasnainvirk

This comment has been minimized.

Contributor

hasnainvirk commented May 21, 2018

@kjbracey-arm Please review again.

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 22, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented May 23, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 23, 2018

Looked like there was a problem device in CI. Restarting test.

/morph test

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 23, 2018

Shuffling around some jobs.

/morph export-build

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 23, 2018

/morph test

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr added ready for merge and removed needs: CI labels May 24, 2018

@cmonr cmonr merged commit 863259e into ARMmbed:master May 24, 2018

13 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 845 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 8972 cycles (+134 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 9964B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label May 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment