-
Notifications
You must be signed in to change notification settings - Fork 3k
LoRaWAN: Adding acquisition of metadata, backoff and a cancel_send() API #6910
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
Conversation
@kivaisan @kjbracey-arm @0xc0170 @cmonr @janjongboom Please review. |
@hasnainvirk can you look at astyle issues - see travis astyle? You can ignore some files that are already on master (will be fixed separately). but for new code, would be nice to align |
features/lorawan/LoRaWANStack.cpp
Outdated
{ | ||
if (_loramac.clear_tx_pipe() == LORAWAN_STATUS_OK) { | ||
if (_device_current_state == DEVICE_STATE_SENDING && | ||
_loramac.get_device_class() == CLASS_A) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why only in class A?
features/lorawan/lorawan_types.h
Outdated
/** | ||
* The transmission time on air of the frame. | ||
*/ | ||
uint32_t tx_toa; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reorder variables to avoid padding
features/lorawan/lorawan_types.h
Outdated
* The SNR for the received packet. | ||
*/ | ||
uint8_t snr; | ||
} lorawan_rx_metadata; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reorder variables to avoid padding
features/lorawan/lorawan_types.h
Outdated
/** | ||
* Frame pending status. | ||
*/ | ||
uint8_t fpending_status; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably fpending bit is not needed. If auto uplink is disabled, stack sends UPLINK_REQUIRED event.
@0xc0170 Style corrections for remaining item could be the topic of another PR. I rectified only those style conflicts pertaining to this PR. |
@kjbracey-arm Can you please review again ? I added a few commits. |
If you're basing on #6901, going to have to wait for its review before any merge of this is possible. |
@kjbracey-arm reviewing doesn't hurt :) I know we have to wait for #6901 |
@0xc0170 This needs CI. Failed for some unrelated issue in the previous run. |
@hasnainvirk We can start CI once this is rebased. Some PRs recently came into master, causing conflicts. A rebase will also relaunch pr-head, whose issues have been resolved earlier today. |
In Class C, rx timeout does not take place for RX2 windows, so if we have not received anything, we would be retrying but if the no. of retries are maxed out, and we have not recieved anything yet, we need a mechanism to tell the upper layer that this has happened.
The scope of style corrections is local to this PR only.
Metadata APIs should return an error if the stack is not initialized yet.
General stability improvements are performed. A flag is added if a Class C RX2 window is open. We shouldn't open it again if its already opened. TX_CRYPTO_ERROR is renamed to CRYPTO_ERROR. Keeping TX_CRYPTO_ERROR for backwards compatibility.
Going to run since CI is just about empty at the moment. Normally we'd wait until at least one reviewer gives the ok, but both mentioned reviewers were already out but the time this PR was ready for review. /morph build |
Build : SUCCESSBuild number : 2146 Triggering tests/morph test |
Test : SUCCESSBuild number : 1940 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need a flag for RX2 window in class C.
As discussed, there are three cases where RX2 is opened:
- When switching from class A to C
- After TX_DONE
- After RX1
So for me it looks like there is a bug if we try to reopen it while it is already open. Please investigate this more.
Exporter Build : SUCCESSBuild number : 1775 |
@hasnainvirk Please take a look at @kivaisan's comment. |
After a discussion with @ChiefBureaucraticOfficer and the fact that Kevin has approved changes and it is unlikely there will be anyone in Finland able to comment on this now before RC1 codefreeze. We will take it in as is. |
@adbridge Understood. |
Description
This PR adds new APIs to our Mbed LoRaWAN stack which facilitate the application
in acquiring metadata relating to transmission or reception as well as an API to cancel
an outstanding outgoing message.
Dependency: #6901
Target: Mbed OS 5.9
Internal ref: IOTCELL-575
Pull request type