-
Notifications
You must be signed in to change notification settings - Fork 131
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
Extract Class A End Device Type functionality into its own class #53
Extract Class A End Device Type functionality into its own class #53
Conversation
@@ -17,6 +17,8 @@ | |||
* | |||
* Author: Davide Magrin <[email protected]> | |||
* Martina Capuzzo <[email protected]> | |||
* | |||
* Modified by: Peggy Anderson <[email protected]> |
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 added this line as this refactor impacts the work flow of how a simulation/program would be run with the module as people will now have to use the ClassAEndDeviceLorawanMac
class instead of the EndDeviceLorawanMac
class.
…wanMac from EndDeviceLorawanMac
…e-lorawan-mac.cc|h
ff888ab
to
ef0da1d
Compare
The fact that this variable was not initialized caused the lengths of receive windows be 0, not allowing the GW to correctly send a message back to the device when ACKs were requested, for instance. Bug found thanks to network-server-example.
Peggy, I completed my review, thank you for this contribution! I was able to spot a bug that caused receive windows to be closed immediately, making downlink communication impossible - however, I easily fixed it by initializing the Other than that this pull request looks good to me, but it also highlighted how we need more tests to ensure the MAC works as expected - I might be able to get some students to work on this, we'll see. We'll keep this on the develop branch for a bit, so that if other issues surface we can take care of them without disturbing the main branch - however you can definitely base your further work upon this contribution, with the certainty that it will make it to master eventually. |
Rebased on develop and pushed, thanks again! |
This pull request works towards issue #39
This pull request fixes issue #54
Proposed Changes
ClassAEndDeviceLorawanMac
class that extendsEndDeviceLorawanMac
EndDeviceLorawanMac
to new classED
toED_A
inlorawan-mac-helper.cc
in anticipation for the addition of Class C EDClassAEndDeviceLorawanMac
EndDeviceLorawanMac
were made protected