Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Uplink MAC state handling fixes #858

Merged
merged 14 commits into from
Jul 2, 2019
Merged

Uplink MAC state handling fixes #858

merged 14 commits into from
Jul 2, 2019

Conversation

rvolosatovs
Copy link
Contributor

@rvolosatovs rvolosatovs commented Jun 27, 2019 •
edited
Loading

Summary

Closes #505
Closes #795
References #42 (closes NS part)
References #789 (closes uplink part)
Rerences #560
Closes #23 (#860)

Changes

  • Derive MAC state(apply MAC commands, account for MAC reset) of the device before computing MIC
  • Add testing utilities for events
  • Reimplement NS uplink testing
  • Harmonize NS testing
  • Do not set ReceivedAt in AS uplink(that's a bug, right? @johanstokking - API states it's set by AS)
  • Do not deduplicate uplinks, which NS cannot process
  • Close AS streams when NS closed
  • Close AS streams in a nicer way
  • Add NS downlink task after uplink at Rx1-nsScheduleWindow
  • Avoid adding NS downlink task if Rx2 is missed
  • Add NS downlink task even if AS did not (yet) process the uplink
  • Allow partial application of events.Definition via events.Definition.BindData and resulting events.DefinitionDataClosure
  • Remove failing tests(will be reverted and fixed in NS uplink testing #870)

Notes for Reviewers

I know the size of this PR is huge, but really most of the changes are NS tests.
If that's a problem - I'm open for suggestions on how to split this up.

@rvolosatovs rvolosatovs added this to the June 2019 milestone Jun 27, 2019
@rvolosatovs rvolosatovs requested a review from htdvisser as a code owner June 27, 2019 08:22
@rvolosatovs rvolosatovs self-assigned this Jun 27, 2019
@coveralls
Copy link

coveralls commented Jun 27, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-0.6%) to 72.495% when pulling bf2b34df5d337ebdf0e28945b69b858d93862b93 on rvolosatovs:feature/abp-resets into 14be53a on TheThingsNetwork:master.

@rvolosatovs rvolosatovs added the blocked This can't continue until another issue or pull request is done label Jun 27, 2019
@rvolosatovs rvolosatovs mentioned this pull request Jun 27, 2019
Merged
@rvolosatovs
Copy link
Contributor Author

Blocked by #860

@rvolosatovs rvolosatovs mentioned this pull request Jun 27, 2019
Merged
@rvolosatovs
Copy link
Contributor Author

rvolosatovs commented Jun 27, 2019 •
edited
Loading

Blocked by #862

@rvolosatovs rvolosatovs mentioned this pull request Jun 27, 2019
Merged
@rvolosatovs rvolosatovs force-pushed the feature/abp-resets branch 7 times, most recently from 3cc9e0f to da0042f Compare June 27, 2019 17:06
@rvolosatovs rvolosatovs mentioned this pull request Jun 27, 2019
Merged
@rvolosatovs rvolosatovs added the blocking release This is blocking a release label Jun 27, 2019
@rvolosatovs rvolosatovs changed the title Network Server uplink fixes and testing Uplink MAC state handling fixes Jun 27, 2019
@rvolosatovs
Copy link
Contributor Author

rvolosatovs commented Jun 27, 2019 •
edited
Loading

I split this up in smaller PRs.
As I mentioned earlier in tech meeting, I removed the failing tests here to make CI pass. That is reverted and fixed in #870

@rvolosatovs rvolosatovs added blocking Another issue or pull request is waiting for this c/network server This is related to the Network Server and removed blocking release This is blocking a release labels Jun 27, 2019
@rvolosatovs rvolosatovs reopened this Jun 28, 2019
@rvolosatovs rvolosatovs changed the base branch from master to feature/ns-uplink June 28, 2019 08:41
@rvolosatovs rvolosatovs removed the request for review from htdvisser June 28, 2019 08:43
@rvolosatovs rvolosatovs removed the blocked This can't continue until another issue or pull request is done label Jun 28, 2019
@johanstokking johanstokking modified the milestones: June 2019, July 2019 Jul 1, 2019
johanstokking
johanstokking approved these changes Jul 1, 2019
View reviewed changes
Copy link
Member

@johanstokking johanstokking left a comment

Choose a reason for hiding this comment

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

LGTM

Bonus points for: Screenshot 2019-07-01 at 18 41 43

@rvolosatovs rvolosatovs merged this pull request into TheThingsNetwork:feature/ns-uplink Jul 2, 2019
@rvolosatovs rvolosatovs deleted the feature/abp-resets branch July 2, 2019 15:30
@rvolosatovs rvolosatovs mentioned this pull request Jul 2, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johanstokking johanstokking johanstokking approved these changes

Assignees

@rvolosatovs rvolosatovs

Labels
blocking Another issue or pull request is waiting for this c/network server This is related to the Network Server
Projects
None yet
Milestone
July 2019
Development

Successfully merging this pull request may close these issues.
3 participants
@rvolosatovs @coveralls @johanstokking