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

Lora: Fix max tx power check #6740

Merged
merged 1 commit into from May 3, 2018

Conversation

Projects
None yet
7 participants
@kivaisan
Contributor

kivaisan commented Apr 25, 2018

Description

In LoRa TX power value 0 means the maximum allowed TX power and values >0
are limiting the allowed TX power to lower.

tx_config was incorrectly checking the power level and causing the maximum
TX power to be always used. Lora gateway can request node to use lower TX
power with LinkAdrReq MAC command.

This fix proposal has been tested with our internal LoRa tests.

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change
Lora: Fix max tx power check
In LoRa TX power value 0 means the maximum allowed TX power and values >0
are limiting the allowed TX power to lower.

tx_config was incorrectly checking the power level and causing the maximum
TX power to be always used. Lora gateway can request node to use lower TX
power with LinkAdrReq MAC command.
@kivaisan

This comment has been minimized.

Contributor

kivaisan commented Apr 25, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 25, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 25, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@kivaisan

This comment has been minimized.

Contributor

kivaisan commented Apr 26, 2018

@0xc0170 could you restart the tests? I don't see these fails to relate this PR.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 26, 2018

/morph test
/morph export-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 27, 2018

/morph test

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 27, 2018

#6740 (comment)

@marcuschangarm Has anything changed in tickers that could cause this failure?

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Apr 27, 2018

That test is supposed to be disabled - see 9f63013. But that has #ifndef MCU_NRF52

But shouldn't that be #ifndef TARGET_MCU_NRF52 (if targets are defined in C? Not sure if they are), or #ifndef NRF52 (which is explicitly set in "macros".).

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Apr 27, 2018

Related issues: #6340 and #6535 - the latter apparently didn't get merged?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Apr 27, 2018

Yes, there is a fix here now #6756 (as noted previous macro was wrong which was a good thing and did not disable the test but rather to find a fix)

@mbed-ci

This comment has been minimized.

@adbridge adbridge added needs: work and removed needs: review labels Apr 30, 2018

@hasnainvirk

This comment has been minimized.

Contributor

hasnainvirk commented May 2, 2018

@adbridge Why does it need work ? It's all set to go in master.

@0xc0170 0xc0170 added ready for merge and removed needs: work labels May 2, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented May 2, 2018

Reviewed, ready for merge

@adbridge

This comment has been minimized.

Contributor

adbridge commented May 2, 2018

I marked it as needs work as there are unanswered questions raised by Kevin on this PR.

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented May 2, 2018

I had no issues - my comments were about the unrelated test failures.

@hasnainvirk

This comment has been minimized.

Contributor

hasnainvirk commented May 3, 2018

@cmonr @0xc0170 Could this be merged please ? I have a PR ready to be made which depends on this.

@0xc0170 0xc0170 merged commit f09ab67 into ARMmbed:master May 3, 2018

12 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/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10282 cycles (+98 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 10112B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@0xc0170 0xc0170 removed the ready for merge label May 3, 2018

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