-
Notifications
You must be signed in to change notification settings - Fork 0
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
net: openthread: Added config options for EUI64 and OUI. #5
base: master
Are you sure you want to change the base?
Conversation
aIeeeEui64[index++] = CONFIG_OPENTHREAD_STACK_VENDOR_OUI & 0xff; | ||
|
||
/* Use device identifier assigned during the production. */ | ||
factoryAddress = (uint64_t)NRF_FICR->DEVICEID[0] << 32; |
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.
The radio is not necessarily the nrf device, this will not work for any other radio.
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.
Agreed, OpenThread just copies the LL address configured on the network interface, if we want to customize it I think it should happen at this level:
https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/ieee802154/ieee802154_nrf5.c#L65
IMO it should be fine to customize eui64 based on the link-layer selected in there.
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.
It was decided to implement such feature only for NRFs, so I moved everything to the nrf Kconfig and ieee802154_nrf5.c file.
aIeeeEui64[index++] = CONFIG_OPENTHREAD_STACK_VENDOR_OUI & 0xff; | ||
|
||
/* Use device identifier assigned during the production. */ | ||
factoryAddress = (uint64_t)NRF_FICR->DEVICEID[0] << 32; |
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.
Agreed, OpenThread just copies the LL address configured on the network interface, if we want to customize it I think it should happen at this level:
https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/ieee802154/ieee802154_nrf5.c#L65
IMO it should be fine to customize eui64 based on the link-layer selected in there.
subsys/net/l2/openthread/Kconfig
Outdated
default 16043574 | ||
help | ||
Custom vendor OUI for OpenThread, | ||
which makes up 24 bits of MAC address |
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.
How about which makes 24 oldest bits of the MAC address
.
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.
Ok, done
subsys/net/l2/openthread/Kconfig
Outdated
config OPENTHREAD_EUI64_CUSTOM_SOURCE | ||
bool "Custom EUI64 source support" | ||
help | ||
Enable providing EUI64 address |
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'd make it explicit in the help message, that otPlatRadioGetIeeeEui64
has to be implemented in that case.
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.
Done
2497ed2
to
c00e9d7
Compare
drivers/ieee802154/ieee802154_nrf5.h
Outdated
@@ -82,4 +82,6 @@ struct nrf5_802154_data { | |||
ieee802154_event_cb_t event_handler; | |||
}; | |||
|
|||
void nrf5_get_eui64(uint8_t *mac); |
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.
should this function really be public? I think you may need to extend driver api for this. calling nrf5_get_eui64 from somewhere not within the nrf5 driver is not allowed.
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.
If the CONFIG_IEEE802154_NRF5_EUI64_CUSTOM_SOURCE is set to y, method has to be implemented by user, so I changed it to be extern and not share API in the header file.
Support for EUI64 address source customization and defining vendor specific OUI (Organizationally Unique Identifier) was added. Signed-off-by: Kamil Kasperczyk <[email protected]>
c00e9d7
to
7dfeda0
Compare
Support for EUI64 address source customization and defining
vendor specific OUI (Organizationally Unique Identifier) was added.
Signed-off-by: Kamil Kasperczyk [email protected]