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

Add constructors for network types. #21

Merged
merged 1 commit into from
Aug 10, 2020
Merged

Add constructors for network types. #21

merged 1 commit into from
Aug 10, 2020

Conversation

newAM
Copy link
Contributor

@newAM newAM commented Aug 9, 2020

This adds constructors for the network types.

Having a const broadcast address is very helpful for building the application layer (e.g. DHCP from issue #16 ).

This does introduce an API change with regards to the names. I chose these names to align with std::net::Ipv4Addr, which may be available in no_std contexts soon (RFC 2832).

I also derived more default traits to be on par with the implementation of Ipv4Addr in smoltcp.

@kellerkindt
Copy link
Owner

Thanks for the link to RFC 2832.
+1 for the const-ness, I think last time I tried that, const fns were not stable yet.
I've never heard or read the name Eui48Addr before. Could you elaborate a bit on that or attach a link to a doc?

@newAM
Copy link
Contributor Author

newAM commented Aug 10, 2020

EUI-48 (Extended Unique Identifiers) is a new term that replaces MAC-48 (source). EUI-48 and MAC-48 differ in name only. I believe the name change was due to the fact that these addresses are now used for other IEEE-802 standards, such as bluetooth and zigbee (source) which use the longer EUI-64.

The name is technically correct, though it is probably needless specificity for this library. Do you think I should change the name back to the original? I do not have a strong opinion on this :)

@kellerkindt
Copy link
Owner

Thanks for the explanation. Although I now understand the reason for the name change, I think I still prefer to just call it MacAddress. I guess as long as this is not a commonly used term (maybe it is and I'm just living under a rock), others will probably stumble over this as I did. Anyway, it would be really neat if you could add a small explanation (as in the first paragraph of your answer) to the struct, explaining why this should be called EUI-48 - to spread the knowledge! :)

@newAM
Copy link
Contributor Author

newAM commented Aug 10, 2020

Sounds good to me! I changed the name back to MacAddress, with a small note about being an EUI-48 MAC address.

@kellerkindt
Copy link
Owner

Thank you!

@kellerkindt kellerkindt merged commit 664e1a0 into kellerkindt:master Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants