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: Reduced priority for automatic uplinks & higher data rate usage for connection establishment #7601

Merged
merged 6 commits into from Aug 7, 2018

Conversation

Projects
None yet
7 participants
@hasnainvirk
Contributor

hasnainvirk commented Jul 26, 2018

Description

This PR handles two issues:

i) User transmissions should have priority over automatic uplinks.
The stack schedules an event for the automatic uplink when needed and the control goes back to
application, if the user stays silent and there is nothing else to do, the automatic uplink goes out as
planned, otherwise user transmission gets priority and automatic uplink gets cancelled.

ii) We should use higher data rates to begin with as they result in smaller transmission times which
doesn't block the channel too long. Plus it also helps getting in synch with the network server faster
which is crucial to set network delays in order to open receive windows.

In addition to that a bug is fixed in post processing of CONFIRMED messages.
As receive timing error and preamle lengths are constants but could be configurable, we have made them configurable and loaded most optimal values as defaults.

Changes in the API and usage is internal to the stack.

Pull request type

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

This comment has been minimized.

Contributor

hasnainvirk commented Jul 26, 2018

@kivaisan Please review.

@hasnainvirk

This comment has been minimized.

Contributor

hasnainvirk commented Jul 26, 2018

@kivaisan Please review again. I have added two more commits to the PR.

@kivaisan

Looks ok.

hasnainvirk added some commits Jul 26, 2018

[IOTCELL-1182] User TX priority over automatic uplinks
In the case when an automatic uplink was queued and the user do have something send,
we should give priority to user data instead od automatic uplink message.
[IOTCELL-1181] Using higher data rates to connect
The stack was trying to connect with default data rates which happened to be
the lowest data rates in a specific region. In the beginning device and NS do
not have agreed upon tx rx parameters and there can be synchronization issues.
When we use lower datarates, we may end up having a minute and a half long
transmissions that hnot only blocks the channel for a long time but also reduce
the chance of proper synch between device and NS. That's why we have decided to
begin with higher data rates and gradually decrease datarate if we do not hear
from the network server.
AS923: Using LoRa modulation comatible DR as default
DR7 is used for FSK in AS932 region. As a default max DR, we should use a LoRa modulation
compatible data rate. Ofcourse if a device wishes to use FSK, it can set DR7 using set_data_rate() API
and turning off ADR, or an NS can configure a new channel for the device utilizing DR7.

@hasnainvirk hasnainvirk changed the title from Reduced priority for automatic uplinks & higher data rate usage for connection establishment to LoRaWAN: Reduced priority for automatic uplinks & higher data rate usage for connection establishment Jul 30, 2018

@hasnainvirk hasnainvirk force-pushed the hasnainvirk:abp_auto_fix branch from 900522e to 57393bc Jul 30, 2018

hasnainvirk added some commits Jul 26, 2018

Mark an error in post processing if ack not recvd
If an ack is not received after maximum retries, we need to mark an error
which was missing from the post processing sequence.
Making recv timing error and preamble length configurable
We had a bug especially in the reception path. Our recv window opening
delays were being calculated on the premise that the radio has to capture
5 preamble symbols out of 8 transmitted by the base station. However, in PHY
layer while setting radio rc settings, we were setting preamble length to be 8.
Preamble length register needs to be configured differently for Uplink and Downlink.
For uplink, we wish to transmit 8 preamble symbols whereas in the reception path we need
to receive 5 preamble symbols at least out of 8.
Alongwith that the maximum range of timing error may vary from platform to platform as it
is based upon the crystal in the chip. We have now made these parameters configurable and
have loaded them with the most optimal defaults.

@hasnainvirk hasnainvirk force-pushed the hasnainvirk:abp_auto_fix branch from 57393bc to c7f3585 Jul 30, 2018

@hasnainvirk

This comment has been minimized.

Contributor

hasnainvirk commented Aug 1, 2018

@SeppoTakalo Can you please review this ?

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Aug 1, 2018

Code looks OK, but I don't know the domain enough to tell whether it is correct, but I assume this was tested already.

@hasnainvirk

This comment has been minimized.

Contributor

hasnainvirk commented Aug 6, 2018

@AnttiKauppila Can you please review this ? Have been in pipeline for a long time now.

@hasnainvirk

This comment has been minimized.

Contributor

hasnainvirk commented Aug 6, 2018

@kjbracey-arm Can you please review ?

@0xc0170 0xc0170 requested review from kjbracey-arm and AnttiKauppila Aug 6, 2018

@AnttiKauppila

LGTM

@0xc0170

0xc0170 approved these changes Aug 6, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Aug 6, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Aug 6, 2018

Build : SUCCESS

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

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.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Aug 7, 2018

/morph mbed2-build

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Aug 7, 2018

@cmonr cmonr merged commit 40ff622 into ARMmbed:master Aug 7, 2018

14 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, 793 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9566 cycles (-104 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9960B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@0xc0170 0xc0170 removed the ready for merge label Aug 7, 2018

pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018

Merge pull request ARMmbed#7601 from hasnainvirk/abp_auto_fix
LoRaWAN: Reduced priority for automatic uplinks & higher data rate usage for connection establishment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment