Skip to content

[csl-tx-scheduler] simplify HandleSentFrame() implementation - #13390

Merged
jwhui merged 1 commit into
openthread:mainfrom
abtink:csl/frame-sent-done
Jul 24, 2026
Merged

[csl-tx-scheduler] simplify HandleSentFrame() implementation#13390
jwhui merged 1 commit into
openthread:mainfrom
abtink:csl/frame-sent-done

Conversation

@abtink

@abtink abtink commented Jul 22, 2026

Copy link
Copy Markdown
Member

This commit simplifies CslTxScheduler::HandleSentFrame() by inlining the helper method directly into the main callback. Since mCslTxNeighbor is already a member variable, passing it as a reference parameter to a helper method was unnecessary.

Additionally, clearing of mCslTxMessage and mCslTxNeighbor is moved to the exit: block after calling HandleSentFrameToCslNeighbor(). Keeping mCslTxMessage non-null while HandleSentFrameToCslNeighbor() runs ensures that nested calls to CslTxScheduler::Update() do not trigger a premature RescheduleCslTx() while the TX done callback is in progress. All state cleanup and rescheduling now occur cleanly at exit:.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the CslTxScheduler::HandleSentFrame() method to streamline its implementation. By inlining the helper function and leveraging the existing mCslTxNeighbor member variable, the code becomes more concise and reduces unnecessary complexity in the control flow.

Highlights

  • Code Simplification: Inlined the helper method HandleSentFrame into the main callback, removing redundant parameter passing.
  • Control Flow Optimization: Consolidated the cleanup of mCslTxNeighbor and the call to RescheduleCslTx() at the exit: label to improve readability and maintainability.
New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request simplifies the CslTxScheduler class by removing the overloaded HandleSentFrame helper method and merging its logic directly into the main HandleSentFrame method. It also refactors the state management of mCslTxNeighbor to be cleared at the end of the operation rather than at the beginning. There are no review comments, and I have no feedback to provide.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

Merging #13390 into main

name branch text data bss total
ot-cli-ftd 54700aa 481696 860 66540 549096
5b93238 481680 860 66540 549080
+/- -16 +0 +0 -16
ot-ncp-ftd 54700aa 447100 764 61776 509640
5b93238 447100 764 61776 509640
+/- +0 +0 +0 +0
ot-cli-mtd 54700aa 373880 764 50932 425576
5b93238 373880 764 50932 425576
+/- +0 +0 +0 +0
ot-ncp-mtd 54700aa 355252 764 46200 402216
5b93238 355252 764 46200 402216
+/- +0 +0 +0 +0
ot-cli-ftd-br 54700aa 593088 864 130684 724636
5b93238 593072 864 130684 724620
+/- -16 +0 +0 -16
ot-rcp 54700aa 63876 568 20828 85272
5b93238 63876 568 20828 85272
+/- +0 +0 +0 +0
Library files
name branch text data bss total
libopenthread-ftd.a 54700aa 251608 95 40319 292022
5b93238 251604 95 40319 292018
+/- -4 +0 +0 -4
libopenthread-cli-ftd.a 54700aa 61777 0 8091 69868
5b93238 61777 0 8091 69868
+/- +0 +0 +0 +0
libopenthread-ncp-ftd.a 54700aa 33359 0 5948 39307
5b93238 33359 0 5948 39307
+/- +0 +0 +0 +0
libopenthread-mtd.a 54700aa 171101 0 24743 195844
5b93238 171101 0 24743 195844
+/- +0 +0 +0 +0
libopenthread-cli-mtd.a 54700aa 41482 0 8059 49541
5b93238 41482 0 8059 49541
+/- +0 +0 +0 +0
libopenthread-ncp-mtd.a 54700aa 25783 0 5948 31731
5b93238 25783 0 5948 31731
+/- +0 +0 +0 +0
libopenthread-ftd-br.a 54700aa 363706 99 104423 468228
5b93238 363702 99 104423 468224
+/- -4 +0 +0 -4
libopenthread-cli-ftd-br.a 54700aa 80892 0 8131 89023
5b93238 80892 0 8131 89023
+/- +0 +0 +0 +0
libopenthread-rcp.a 54700aa 9946 0 5060 15006
5b93238 9946 0 5060 15006
+/- +0 +0 +0 +0
libopenthread-radio.a 54700aa 20763 0 262 21025
5b93238 20763 0 262 21025
+/- +0 +0 +0 +0

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.73%. Comparing base (54700aa) to head (cb2baf0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/core/thread/csl_tx_scheduler.cpp 65.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13390      +/-   ##
==========================================
+ Coverage   68.82%   73.73%   +4.91%     
==========================================
  Files         711      712       +1     
  Lines       98565    97385    -1180     
==========================================
+ Hits        67835    71808    +3973     
+ Misses      30730    25577    -5153     
Files with missing lines Coverage Δ
src/core/thread/csl_tx_scheduler.hpp 94.11% <ø> (-5.89%) ⬇️
src/core/thread/csl_tx_scheduler.cpp 83.05% <65.00%> (-7.71%) ⬇️

... and 200 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@abtink
abtink marked this pull request as ready for review July 23, 2026 00:29
Comment thread src/core/thread/csl_tx_scheduler.cpp
@abtink
abtink force-pushed the csl/frame-sent-done branch from 1905dc6 to acc8d32 Compare July 23, 2026 17:52
@abtink

abtink commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the CslTxScheduler class to simplify frame transmission handling. Specifically, it merges the two overloaded HandleSentFrame methods into a single method, directly utilizing mCslTxNeighbor and moving the cleanup and rescheduling logic to the exit block. Additionally, it updates CslTxScheduler::Update to exit early when there is no active CSL TX message and improves code readability with additional comments and simplified checks. There are no review comments to evaluate, and I have no additional feedback to provide on these changes.

This commit simplifies `CslTxScheduler::HandleSentFrame()` by inlining
the helper method directly into the main callback. Since `mCslTxNeighbor`
is already a member variable, passing it as a reference parameter to a
helper method was unnecessary.

Additionally, clearing of `mCslTxMessage` and `mCslTxNeighbor` is moved
to the `exit:` block after calling `HandleSentFrameToCslNeighbor()`.
Keeping `mCslTxMessage` non-null while `HandleSentFrameToCslNeighbor()`
runs ensures that nested calls to `CslTxScheduler::Update()` do not
trigger a premature `RescheduleCslTx()` while the TX done callback is in
progress. All state cleanup and rescheduling now occur cleanly at `exit:`.
@abtink
abtink force-pushed the csl/frame-sent-done branch from acc8d32 to cb2baf0 Compare July 24, 2026 01:16
@jwhui
jwhui merged commit 211267d into openthread:main Jul 24, 2026
94 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.

2 participants