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.

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

Support migration from Firefly #49

Closed
wants to merge 14 commits into from

Conversation

happyRip
Copy link
Member

Summary

Closes #43

Changes

  • Add firefly source

Testing

...

Regressions

...

Notes for Reviewers

nil

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Documentation: Relevant documentation is added or updated.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.

@happyRip happyRip linked an issue Sep 14, 2022 that may be closed by this pull request
@happyRip happyRip marked this pull request as draft September 14, 2022 12:30
@happyRip happyRip changed the title Feat/43 support firefly migration Support migration from Firefly Sep 14, 2022
@happyRip happyRip force-pushed the feat/43-support-firefly-migration branch from 055d8b0 to 216881e Compare September 16, 2022 14:14
@happyRip happyRip force-pushed the feat/43-support-firefly-migration branch 2 times, most recently from 1f7a246 to cba0aa6 Compare September 20, 2022 14:38
Copy link
Contributor

@adriansmares adriansmares left a comment

Choose a reason for hiding this comment

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

Looking great !

pkg/source/firefly/source.go Outdated Show resolved Hide resolved
pkg/source/firefly/source.go Show resolved Hide resolved
v3dev.Session.LastFCntUp = uint32(packet.FCnt)
}

if !s.config.dryRun {
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

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.

@happyRip happyRip force-pushed the feat/43-support-firefly-migration branch from cba0aa6 to 06a7ac8 Compare September 20, 2022 15:25
@johanstokking
Copy link
Member

What is the status here?

@happyRip
Copy link
Member Author

It's blocked since we lost access to a Firefly instance.

@KrishnaIyer
Copy link
Member

replaced by #97

@KrishnaIyer KrishnaIyer closed this Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support migration from Firefly
4 participants