Skip to content

[tlvs] add Start/End/AdjustTlv methods for staged writing - #12250

Merged
jwhui merged 1 commit into
openthread:mainfrom
abtink:tlv/start-end-tlv
Dec 31, 2025
Merged

[tlvs] add Start/End/AdjustTlv methods for staged writing#12250
jwhui merged 1 commit into
openthread:mainfrom
abtink:tlv/start-end-tlv

Conversation

@abtink

@abtink abtink commented Dec 30, 2025

Copy link
Copy Markdown
Member

This commit introduces a new set of static methods to simplify writing TLVs with variable lengths to a Message.

The new mechanism consists of three methods:

  • Tlv::StartTlv(): Appends a placeholder TLV header and returns a Bookmark.
  • Tlv::AdjustTlv(): Optionally promotes the TLV to an extended TLV if the length grows beyond the standard TLV limit. This is an optimization to avoid large copies within a message.
  • Tlv::EndTlv(): Calculates the final length and updates the TLV header, promoting to an extended TLV if necessary.

This new set replaces the common but cumbersome pattern of manually saving the start offset, appending data, and then back-patching the length field.

The existing code is updated to use this new, simpler, and more robust mechanism.

This commit also adds unit tests to validate the new functionality.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @abtink, 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 significantly enhances the handling of Type-Length-Value (TLV) structures within messages by introducing a new set of static methods for staged writing. This change streamlines the process of appending variable-length TLVs, eliminating the need for manual length calculation and back-patching. The new methods improve code clarity, reduce potential errors, and offer performance optimizations for large TLVs, ultimately making TLV serialization more efficient and maintainable across the codebase.

Highlights

  • New Staged TLV Writing Methods: Introduced Tlv::StartTlv(), Tlv::AdjustTlv(), and Tlv::EndTlv() to simplify writing variable-length TLVs to a Message object.
  • Improved TLV Handling: The new mechanism replaces a common, manual, and error-prone pattern of back-patching length fields, making TLV serialization more robust.
  • Performance Optimization: AdjustTlv() allows for optional promotion of a TLV to an extended TLV during writing, which helps avoid potentially large memory copies at the final EndTlv() call.
  • Code Refactoring: Existing codebases, including discover_scanner.cpp, link_metrics.cpp, mle.cpp, mle_ftd.cpp, and network_diagnostic.cpp, have been updated to utilize these new, more efficient methods.
  • Unit Tests: Comprehensive unit tests have been added to validate the correctness and behavior of the new TLV writing functionality, covering various scenarios for standard and extended TLVs.

🧠 New Feature in Public Preview: You can now enable Memory 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 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 counter productive. 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.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

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 introduces new helper functions (StartTlv, AdjustTlv, EndTlv) within the Tlv class to streamline the process of appending TLVs to messages, particularly when the TLV's final length is unknown during initial creation. These functions manage the creation of placeholder TLV headers, dynamically promote standard TLVs to extended TLVs if their content exceeds the base length limit (with AdjustTlv offering an optional optimization to avoid large memory copies), and finalize the TLV by writing the correct length. Existing code in discover_scanner.cpp, link_metrics.cpp, mle.cpp, and network_diagnostic.cpp has been refactored to utilize this new pattern, replacing manual TLV header management and length calculations. Additionally, comprehensive unit tests have been added in test_tlv.cpp to validate the functionality of these new TLV appending methods.

@github-actions

github-actions Bot commented Dec 30, 2025

Copy link
Copy Markdown

Merging #12250 into main

name branch text data bss total
ot-cli-ftd ed62353 482304 860 66620 549784
1f701de 482208 860 66620 549688
+/- -96 +0 +0 -96
ot-ncp-ftd ed62353 448100 764 61856 510720
1f701de 448004 764 61856 510624
+/- -96 +0 +0 -96
ot-cli-mtd ed62353 373424 764 51052 425240
1f701de 373536 764 51052 425352
+/- +112 +0 +0 +112
ot-ncp-mtd ed62353 354548 764 46320 401632
1f701de 354644 764 46320 401728
+/- +96 +0 +0 +96
ot-cli-ftd-br ed62353 589632 868 136532 727032
1f701de 589504 868 136532 726904
+/- -128 +0 +0 -128
ot-rcp ed62353 63264 568 20804 84636
1f701de 63264 568 20804 84636
+/- +0 +0 +0 +0
Library files
name branch text data bss total
libopenthread-ftd.a ed62353 249360 95 40390 289845
1f701de 249268 95 40390 289753
+/- -92 +0 +0 -92
libopenthread-cli-ftd.a ed62353 61833 0 8091 69924
1f701de 61833 0 8091 69924
+/- +0 +0 +0 +0
libopenthread-ncp-ftd.a ed62353 33581 0 5948 39529
1f701de 33581 0 5948 39529
+/- +0 +0 +0 +0
libopenthread-mtd.a ed62353 166568 0 24854 191422
1f701de 166630 0 24854 191484
+/- +62 +0 +0 +62
libopenthread-cli-mtd.a ed62353 41426 0 8059 49485
1f701de 41426 0 8059 49485
+/- +0 +0 +0 +0
libopenthread-ncp-mtd.a ed62353 25805 0 5948 31753
1f701de 25805 0 5948 31753
+/- +0 +0 +0 +0
libopenthread-ftd-br.a ed62353 358332 100 110262 468694
1f701de 358212 100 110262 468574
+/- -120 +0 +0 -120
libopenthread-cli-ftd-br.a ed62353 80246 0 8131 88377
1f701de 80246 0 8131 88377
+/- +0 +0 +0 +0
libopenthread-rcp.a ed62353 9944 0 5060 15004
1f701de 9944 0 5060 15004
+/- +0 +0 +0 +0
libopenthread-radio.a ed62353 19547 0 238 19785
1f701de 19547 0 238 19785
+/- +0 +0 +0 +0

@codecov

codecov Bot commented Dec 30, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.28058% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.90%. Comparing base (ed62353) to head (d4cd40b).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/core/thread/network_diagnostic.cpp 95.23% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12250      +/-   ##
==========================================
- Coverage   75.91%   72.90%   -3.02%     
==========================================
  Files         681      680       -1     
  Lines       95588    99198    +3610     
==========================================
- Hits        72564    72317     -247     
- Misses      23024    26881    +3857     
Files with missing lines Coverage Δ
src/core/common/tlvs.cpp 99.36% <100.00%> (+0.15%) ⬆️
src/core/common/tlvs.hpp 100.00% <ø> (ø)
src/core/meshcop/tcat_agent.cpp 60.81% <100.00%> (-25.81%) ⬇️
src/core/thread/discover_scanner.cpp 84.52% <100.00%> (-0.28%) ⬇️
src/core/thread/link_metrics.cpp 88.45% <100.00%> (-0.17%) ⬇️
src/core/thread/mle.cpp 89.30% <100.00%> (-0.66%) ⬇️
src/core/thread/mle_ftd.cpp 86.97% <100.00%> (-0.17%) ⬇️
tests/unit/test_tlv.cpp 100.00% <100.00%> (ø)
src/core/thread/network_diagnostic.cpp 90.13% <95.23%> (+4.66%) ⬆️

... and 192 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This commit introduces a new set of static methods to simplify
writing TLVs with variable lengths to a `Message`.

The new mechanism consists of three methods:
- `Tlv::StartTlv()`: Appends a placeholder TLV header and returns a
  `Bookmark`.
- `Tlv::AdjustTlv()`: Optionally promotes the TLV to an extended TLV
  if the length grows beyond the standard TLV limit. This is an
  optimization to avoid large copies within a message.
- `Tlv::EndTlv()`: Calculates the final length and updates the TLV
  header, promoting to an extended TLV if necessary.

This new set replaces the common but cumbersome pattern of manually
saving the start offset, appending data, and then back-patching the
length field.

The existing code is updated to use this new, simpler, and more
robust mechanism.

This commit also adds unit tests to validate the new functionality.
@abtink
abtink marked this pull request as ready for review December 30, 2025 19:02
@jwhui
jwhui merged commit da1f792 into openthread:main Dec 31, 2025
120 of 122 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