Skip to content

Conversation

@ryankurte
Copy link
Member

hi there, thanks for all your work on this it's great for getting going with 802.15.4. i've been working on some MACs over at rust-iot/rust-lpwan and have needed both 802.15.4-2015 and defmt support.

this PR adds a few missing fields towards the former and an opt-in feature for the latter.

still some work to do, also i have yet to implement IE encoding / decoding and i was wondering whether you would be happy with that here too?

@ryankurte ryankurte force-pushed the feature/802.15.4-2015 branch from 13d87ea to 176fec5 Compare April 27, 2021 23:02
@ryankurte
Copy link
Member Author

huh, the security implementation (while super neat!) has made using this library as a set of components as i was much more difficult. i will have to stick with 0.3.0 for now / probably won't continue this PR as it's too much work to update from my side so, feel free to merge or discard it as you see fit.

@hannobraun
Copy link
Contributor

Hey @ryankurte, thank you for the PR!

i have yet to implement IE encoding / decoding and i was wondering whether you would be happy with that here too?

I have no idea what that is. If it's part of IEEE 802.15.4, then I guess it makes sense to have in this crate. (Context: I initially wrote this crate, but haven't really worked with IEEE 802.15.4 since. I do my best to keep up with pull requests and releases, but rely on contributors for all the domain knowledge.)

huh, the security implementation (while super neat!) has made using this library as a set of components as i was much more difficult. i will have to stick with 0.3.0 for now

That doesn't sound good! If you're up for it, I'd appreciate an issue that describes what specifically is the problem, and maybe ideas for how it can be fixed. And of course, I'd appreciate any PRs that improve on the situation, be it from you or anyone else.

probably won't continue this PR as it's too much work to update from my side so, feel free to merge or discard it as you see fit.

I've taken a look at this PR. The defmt support looks good, I'd love to merge that. I don't see anything wrong with the rest either, but lack the bandwidth to really understand it or check it against the spec. Does it make sense to merge as-is, in your opinion? Or does it require more work to be merge-ready, as you implied?

@ryankurte
Copy link
Member Author

I have no idea what that is. If it's part of IEEE 802.15.4, ..

sorry, Information Elements, part of the 2015 spec for time sync'd channel hopping.

I'd appreciate an issue that describes what specifically is the problem

if i can find time to look at refactoring my work around it again i'll see what i can do ^_^

Does it make sense to merge as-is, in your opinion? Or does it require more work to be merge-ready, as you implied?

it's not close to a complete 2015 implementation which was what i was aiming for originally, but the small additions are useful of themselves, i'd probably be inclined to merge it as useful if incomplete.

@hannobraun
Copy link
Contributor

Thank you for the additional info, @ryankurte. I'd be inclined to merge it then.

CI is failing though, so that will require fixing first. I understand if you don't want to look into it, of course, so anyone else reading this, feel free to pick this up and make a follow-up PR!

@hannobraun hannobraun added the status: waiting on author Pull request requires changes by the author label May 3, 2021
@korken89 korken89 mentioned this pull request Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: waiting on author Pull request requires changes by the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants