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

drivers: ieee802154: New API for modulated carrier wave transmission. #82134

Merged
merged 1 commit into from
Nov 29, 2024

Conversation

canisLupus1313
Copy link
Contributor

Added new API function to start modulated carrier wave transmission

ankuns
ankuns previously approved these changes Nov 27, 2024
rlubos
rlubos previously approved these changes Nov 27, 2024
@@ -1786,6 +1786,29 @@ struct ieee802154_radio_api {
*/
int (*continuous_carrier)(const struct device *dev);

/**
Copy link
Member

Choose a reason for hiding this comment

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

It might be good to surround this with a Kconfig, if it is not normally available during regular device operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cfriedt Both continuous_carrier and modulated_carrier on the nRF platform driver require extra Kconfig CONFIG_NRF_802154_CARRIER_FUNCTIONS but I don't see why other platforms would have to follow the same logic. So I would prefer to leave it as is.

Copy link
Member

Choose a reason for hiding this comment

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

Why not simply have a generic carrier Kconfig option here and apply the same one in the Nordic driver (or both?)

Additional reasons:

why expose a NULL function pointer to API consumers?

If a device isn't built for test and is never intended to transmit CW, then code won't accidentally compile or call it, or worse, make that functionality available in the field, possibly jamming other devices?

Zephyr should be minimal by default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please take a look at updated version.

drivers/ieee802154/Kconfig Outdated Show resolved Hide resolved
Added new API function to start modulated carrier wave transmission

Signed-off-by: Przemyslaw Bida <[email protected]>
@@ -204,7 +204,7 @@ config NRF_802154_SECURITY_KEY_STORAGE_SIZE

config NRF_802154_CARRIER_FUNCTIONS
bool "nRF 802.15.4 carrier functions"
default y if OPENTHREAD_DIAG
default y if (OPENTHREAD_DIAG || IEEE802154_CARRIER_FUNCTIONS)
Copy link
Member

Choose a reason for hiding this comment

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

as OPENTHREAD_DIAG now depends on IEEE802154_CARRIER_FUNCTIONS isn't this or statement redundant? We will never have a situation when diag is enabled but carrier functions is disabled. so maybe just default y if IEEE802154_CARRIER_FUNCTIONS would be enough?

@kartben kartben merged commit e0f94f8 into zephyrproject-rtos:main Nov 29, 2024
27 checks passed
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.

10 participants