Skip to content

pkg/semtech-loramac: add link check support#8639

Merged
jia200x merged 2 commits intoRIOT-OS:masterfrom
aabadie:pr/pkg/loramac_link_check
May 8, 2018
Merged

pkg/semtech-loramac: add link check support#8639
jia200x merged 2 commits intoRIOT-OS:masterfrom
aabadie:pr/pkg/loramac_link_check

Conversation

@aabadie
Copy link
Copy Markdown
Contributor

@aabadie aabadie commented Feb 26, 2018 •
edited
Loading

Contribution description

This PR adds support for the LoRaWAN "LinkCheckReq" MAC command.

The feature adds the possibility to validate the connectivity with the network by retrieving 2 informations:

  • the number of gateways reachable by the end device
  • the demodulation margin indicates the link margin in dB of the last received command (from the specs:
    "A value of 0 means that the frame was received at the demodulation floor (0 dB or no
    margin) while a value of 20, for example, means that the frame reached the gateway 20 dB
    above the demodulation floor"

The link check request will be sent with the next TX packet and the reply will be received by the nodes in the following RX window. This means that one has to schedule the request and check for the result after a TX.

This PR also adds the link_check subcommand to the loramac command of the pkg_semtech-loramac test application shell. Use it as follows:

# schedule the link check
> loramac link_check
Link check request scheduled
# then retrieve the link check information
> loramac tx test
TX done
Link check information:
- Demodulation margin: 38
- Number of gateways: 1

As a side note, I'm planning to refactor the API of the semtech loramac package by introducing a structure that will handle a loramac instance. This structure will also contain usefull attributes (radio device descriptor, keys, rx_data, etc) and will simplify the netdev adaption (I think) and help fixing thread safety issues. see #8798

Issues/PRs references

Now based on #8798

@aabadie aabadie added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: LoRa Area: LoRa radio support labels Feb 26, 2018
@aabadie aabadie requested a review from jia200x February 26, 2018 08:19
@aabadie aabadie changed the title Pr/pkg/loramac link check pkg/semtech-loramac: add link check support Feb 26, 2018
jia200x
jia200x requested changes Feb 26, 2018
View reviewed changes
pkg/semtech-loramac/contrib/semtech_loramac.c
_semtech_loramac_link_check_received = false;
MlmeReq_t mlmeReq;
mlmeReq.Type = MLME_LINK_CHECK;
LoRaMacMlmeRequest(&mlmeReq);
Copy link
Copy Markdown
Member

@jia200x jia200x Feb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would try to use here something similar to the mlme_confirm function. That is, sending a message to the LoRaMAC thread to process this MLME request. Otherwise, this won't be thread safe.

Copy link
Copy Markdown
Member

@jia200x jia200x Feb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Since in this case, this function is called from main thread)

Copy link
Copy Markdown
Contributor Author

@aabadie aabadie Feb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that problem, it's the same story with the get/set functions. But for the moment, it's fine I think, until I finalize the refactoring I'm working on (as described in the initial comment).

Copy link
Copy Markdown
Member

@jia200x jia200x Feb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this branch is helpful for you. It's an abandoned branch though

Copy link
Copy Markdown
Contributor Author

@aabadie aabadie Feb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it helps certainly! That's indeed in the spirit of what I have in mind.

@jia200x
Copy link
Copy Markdown
Member

jia200x commented Feb 26, 2018

As a side note, I'm planning to refactor the API of the semtech loramac package by introducing a structure that will handle a loramac instance. This structure will also contain usefull attributes (radio device descriptor, keys, rx_data, etc) and will simplify the netdev adaption (I think) and help fixing thread safety issues.

That would be awesome

@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Mar 19, 2018

That would be awesome

Here it is: #8798. I still need to adapt this one.

@aabadie aabadie force-pushed the pr/pkg/loramac_link_check branch from a820de0 to 41da691 Compare March 19, 2018 22:16
@aabadie aabadie added the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 19, 2018
@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Mar 21, 2018

I still need to adapt this one.

Done and tested.

@aabadie aabadie force-pushed the pr/pkg/loramac_link_check branch 5 times, most recently from a3676ee to 135d321 Compare March 29, 2018 17:27
@aabadie aabadie mentioned this pull request Mar 30, 2018
Merged
@aabadie aabadie force-pushed the pr/pkg/loramac_link_check branch 2 times, most recently from 30fcba4 to b09d092 Compare April 2, 2018 16:02
@aabadie aabadie mentioned this pull request Apr 2, 2018
Merged
@jia200x jia200x mentioned this pull request Apr 9, 2018
Merged
@aabadie aabadie mentioned this pull request Apr 19, 2018
Merged
@aabadie aabadie force-pushed the pr/pkg/loramac_link_check branch from b09d092 to 04831e8 Compare April 19, 2018 11:29
@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Apr 19, 2018 •
edited
Loading

@jia200x, next one is here ;) and it is now rebased on latest master.

@jia200x
Copy link
Copy Markdown
Member

jia200x commented Apr 19, 2018

@aabadie tested and works good.

The only issue I see is the fact the link_check command is not being show in the usage:

2018-04-19 14:41:30,338 - INFO #  loramac
2018-04-19 14:41:30,341 - INFO # Usage: loramac <get|set|join|tx>

But works as expected!

@aabadie aabadie force-pushed the pr/pkg/loramac_link_check branch from 04831e8 to aa0acd6 Compare April 19, 2018 13:51
@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Apr 19, 2018

The only issue I see is the fact the link_check command is not being show in the usage

Ah yes, good catch! It should be fixed now

@aabadie aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 19, 2018
@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Apr 25, 2018

@jia200x, do you still see blocking issues with this one ?

@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented May 7, 2018

ping @jia200x

@jia200x
Copy link
Copy Markdown
Member

jia200x commented May 8, 2018

Looks good! It's fine to me

@jia200x
Copy link
Copy Markdown
Member

jia200x commented May 8, 2018

ACK & GO

@jia200x jia200x merged commit 736c757 into RIOT-OS:master May 8, 2018
@aabadie aabadie deleted the pr/pkg/loramac_link_check branch June 1, 2018 19:03
@cladmi cladmi added this to the Release 2018.07 milestone Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jia200x jia200x jia200x approved these changes

Assignees

No one assigned

Labels

Area: LoRa Area: LoRa radio support CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Milestone

Release 2018.07

Development

Successfully merging this pull request may close these issues.

3 participants

@aabadie @jia200x @cladmi