Skip to content

Conversation

@morigawa
Copy link
Contributor

Contribution description

The OpenThread example didn't work for nrf52840 based boards (adafruit feather sense, nrf52840dk). The problem is that the bytes_tx value in the submac netdev (established in #20992), was not working correctly. It only could return 0 or a negative value for errors. bytes_tx should return the length of the packet or an error. Because bytes_tx is only evaluated after a TX_DONE event, it shouldn't return 0 in a reasonable case. If this for some reason does happen, otPlatRadioTxDone should be called with OT_ERROR_ABORT instead of OT_ERROR_FAILED. In the case of OT_ERROR_FAILED, "unreachable" code is reached and this causes a panic.

Testing procedure

Because there is currently a problem with the nrf driver in master (#21782), I cherry-picked the commits onto a previous state and tested them on a nrf52840dk.

Issues/PRs references

See also #20992
Depends on #21786

@morigawa morigawa requested a review from jia200x as a code owner October 15, 2025 11:24
@github-actions github-actions bot added Area: network Area: Networking Area: pkg Area: External package ports Area: drivers Area: Device drivers labels Oct 15, 2025
@morigawa morigawa changed the title Pr/netdev ieee802154 submac fix bytestx drivers/netdev_ieee802154_submac: fix bytes_tx Oct 15, 2025
@crasbe crasbe added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) State: waiting for other PR State: The PR requires another PR to be merged first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 15, 2025
@crasbe crasbe requested a review from fabian18 October 15, 2025 11:43
@riot-ci
Copy link

riot-ci commented Oct 15, 2025

Murdock results

✔️ PASSED

073d165 pkg/openthread: change error code

Success Failures Total Runtime
10560 0 10560 12m:48s

Artifacts

@fabian18
Copy link
Contributor

There is a size_t iolist_size(const iolist_t *iolist);

@fabian18
Copy link
Contributor

Looks good I think you can squash

@morigawa morigawa force-pushed the pr/netdev_ieee802154_submac_fix_bytestx branch from b456d7f to ebb4881 Compare October 23, 2025 10:33
@benpicco benpicco removed the State: waiting for other PR State: The PR requires another PR to be merged first label Oct 31, 2025
@benpicco benpicco added this to the Release 2025.10 milestone Oct 31, 2025
@benpicco benpicco enabled auto-merge October 31, 2025 10:31
@benpicco
Copy link
Contributor

Looks like this needs a rebase onto the latest master to fix static tests.

@crasbe crasbe added the State: needs rebase State: The codebase was changed since the creation of the PR, making a rebase necessary label Oct 31, 2025
bytes_tx should not be set to 0 at this point. It should be set to the
length of the package, if there is an error after transmission it will
be set to an error value in submac_tx_done. The value will be evaluated
after a TX_DONE Event, so the pending state 0 that is returned from the
_send function while async sending, is not relevant for bytes_tx.
Using OT_ERROR_FAILED let to unreachable code and following that a panic.
auto-merge was automatically disabled October 31, 2025 19:42

Head branch was pushed to by a user without write access

@morigawa morigawa force-pushed the pr/netdev_ieee802154_submac_fix_bytestx branch from ebb4881 to 073d165 Compare October 31, 2025 19:42
@crasbe crasbe removed the State: needs rebase State: The codebase was changed since the creation of the PR, making a rebase necessary label Oct 31, 2025
@crasbe crasbe enabled auto-merge October 31, 2025 20:05
@crasbe crasbe added this pull request to the merge queue Oct 31, 2025
Merged via the queue into RIOT-OS:master with commit 12b719a Oct 31, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: drivers Area: Device drivers Area: network Area: Networking Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants