-
Notifications
You must be signed in to change notification settings - Fork 3
Support migration from Firefly #49
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
055d8b0 to
216881e
Compare
1f7a246 to
cba0aa6
Compare
adriansmares
left a comment
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.
Looking great !
| v3dev.Session.LastFCntUp = uint32(packet.FCnt) | ||
| } | ||
|
|
||
| if !s.config.dryRun { |
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 am inclined to instead have a --clear-keys instead. In retrospective, lots of people broke their end devices with ttnv2 by not knowing that outside of dry runs, the keys are cleared.
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.
Making it safe by default seems a good idea to me. --clear-keys seems to be related to the implementation, maybe something describing what's happening from a higher point of view like --disable-devices (probably some better options out there) could work as a universal solution in the future? Unless that's not the intent.
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.
Disable implies that it can be rolled back, which is not really the case when you delete the keys and forget to save the output (this happened many times). At least --clear-keys is equivalent to --i-really-know-what-i-am-doing.
cba0aa6 to
06a7ac8
Compare
|
What is the status here? |
|
It's blocked since we lost access to a Firefly instance. |
|
replaced by #97 |
Summary
Closes #43
Changes
fireflysourceTesting
...
Regressions
...
Notes for Reviewers
nil
Checklist
CHANGELOG.md.