Skip to content

ieee802154/submac: calculate symbol time on demand#19668

Merged
benpicco merged 6 commits intoRIOT-OS:masterfrom
benpicco:ieee802154_symbol_time_on_demand
Feb 20, 2025
Merged

ieee802154/submac: calculate symbol time on demand#19668
benpicco merged 6 commits intoRIOT-OS:masterfrom
benpicco:ieee802154_symbol_time_on_demand

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented May 25, 2023 •
edited
Loading

Contribution description

This moves the ACK timeout/CSMA backoff period calculation from the at86rf215 driver to the radio HAL so it can be re-used by other IEEE 802.15.4g drivers.

To do so the ieee802154_phy_conf_t structure is extended depending on the used modulation. This allows to add alternative modulations with different properties (e.g. #19172).

Testing procedure

Currently there is no driver that makes use of this.

Issues/PRs references

alternative to #19198

@benpicco benpicco requested review from jia200x and kYc0o May 25, 2023 14:05
@github-actions github-actions bot added Area: drivers Area: Device drivers Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework labels May 25, 2023
@benpicco benpicco force-pushed the ieee802154_symbol_time_on_demand branch 2 times, most recently from 5377f37 to bd982d4 Compare May 25, 2023 16:06
@benpicco benpicco marked this pull request as ready for review May 25, 2023 16:10
@benpicco benpicco force-pushed the ieee802154_symbol_time_on_demand branch from bd982d4 to d2b99fc Compare May 25, 2023 16:11
@benpicco benpicco mentioned this pull request May 25, 2023
Closed
@benpicco benpicco requested a review from jeandudey May 25, 2023 16:12
@benpicco benpicco force-pushed the ieee802154_symbol_time_on_demand branch from d2b99fc to 6fc9722 Compare May 25, 2023 16:42
kfessel
kfessel reviewed May 26, 2023
View reviewed changes
@benpicco benpicco force-pushed the ieee802154_symbol_time_on_demand branch from 6fc9722 to 494bb3b Compare June 5, 2023 12:33
@benpicco benpicco requested review from dylad and keestux as code owners June 5, 2023 12:33
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Jun 5, 2023
@benpicco benpicco force-pushed the ieee802154_symbol_time_on_demand branch from f135bbf to b248208 Compare February 19, 2024 18:23
@maribu
Copy link
Copy Markdown
Member

maribu commented Feb 19, 2024

This seems to need some MAYBE_UNUSED being sprinkled over some function declarations.

@benpicco benpicco force-pushed the ieee802154_symbol_time_on_demand branch from b248208 to 2131868 Compare February 19, 2024 21:32
@benpicco benpicco requested review from jia200x and removed request for jia200x February 27, 2024 14:56
@benpicco benpicco requested a review from kfessel March 12, 2024 14:34
kfessel
kfessel reviewed Mar 12, 2024
View reviewed changes
kfessel
kfessel reviewed Mar 12, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

looks good (better than before) and seems to pass the exhaustive test i will run that test again and if it succeeds i will approve unless someone else has objections (within reasonable time )

fabian18
fabian18 reviewed Dec 9, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@fabian18 fabian18 left a comment

Choose a reason for hiding this comment

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

I would like to understand the calculations. IEEE 802.15.4g-2012 should be sufficient, or do you recall any other document, because I think I have to figure out something similar for LoRa.

I'm glad to have this already.

fabian18
fabian18 reviewed Dec 9, 2024
View reviewed changes
@fabian18 fabian18 mentioned this pull request Feb 10, 2025
Open
maribu
maribu reviewed Feb 10, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

Looks good to me. There are still some open points from @fabian18's comments.

@benpicco benpicco force-pushed the ieee802154_symbol_time_on_demand branch from 2379296 to d977217 Compare February 10, 2025 13:52
@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Feb 10, 2025

Thank you for the ping!

@benpicco benpicco force-pushed the ieee802154_symbol_time_on_demand branch from d977217 to ef72aae Compare February 10, 2025 13:56
@kYc0o
Copy link
Copy Markdown
Contributor

kYc0o commented Feb 12, 2025

Does this need any more testing?

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Feb 17, 2025

looks like some testing was already provided in #21202 😃

@maribu
Copy link
Copy Markdown
Member

maribu commented Feb 17, 2025

@fabian18 Could you take another look?

fabian18
fabian18 reviewed Feb 19, 2025
View reviewed changes
@benpicco benpicco force-pushed the ieee802154_symbol_time_on_demand branch from ef72aae to 73e45dd Compare February 20, 2025 11:00
fabian18
fabian18 approved these changes Feb 20, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@fabian18 fabian18 left a comment

Choose a reason for hiding this comment

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

looks like some testing was already provided in #21202 😃

Worked well for me.

Does this need any more testing?

For the multi rate modulation calculations I would trust benpiccos knowledge and experience with the at86rf215 transceiver.

@benpicco benpicco added this pull request to the merge queue Feb 20, 2025
Merged via the queue into RIOT-OS:master with commit f67055c Feb 20, 2025
26 checks passed
@benpicco benpicco deleted the ieee802154_symbol_time_on_demand branch February 20, 2025 15:24
@mguetschow mguetschow added this to the Release 2025.04 milestone Apr 8, 2025
@fabian18 fabian18 mentioned this pull request Jun 3, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kfessel kfessel kfessel left review comments

@maribu maribu maribu left review comments

@fabian18 fabian18 fabian18 approved these changes

@kYc0o kYc0o Awaiting requested review from kYc0o

@smlng smlng Awaiting requested review from smlng

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

@cgundogan cgundogan Awaiting requested review from cgundogan

@jeandudey jeandudey Awaiting requested review from jeandudey

@dylad dylad Awaiting requested review from dylad

@keestux keestux Awaiting requested review from keestux

@jia200x jia200x Awaiting requested review from jia200x

Assignees

No one assigned

Labels

Area: drivers Area: Device drivers Area: network Area: Networking Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR

Projects

None yet

Milestone

Release 2025.04

Development

Successfully merging this pull request may close these issues.

7 participants

@benpicco @riot-ci @maribu @kYc0o @kfessel @fabian18 @mguetschow