Skip to content

[network-diag] simplify ChannelPagesTlv and diag data parsing - #12706

Merged
jwhui merged 1 commit into
openthread:mainfrom
abtink:diag/channel-page-tlv
Mar 17, 2026
Merged

[network-diag] simplify ChannelPagesTlv and diag data parsing#12706
jwhui merged 1 commit into
openthread:mainfrom
abtink:diag/channel-page-tlv

Conversation

@abtink

@abtink abtink commented Mar 17, 2026

Copy link
Copy Markdown
Member

This commit simplifies appending ChannelPagesTlv using the standard Tlv::Append<>() with the the array of supported channel pages as the TLV value.

In addition, a ReadDiagData() helper method is introduced in the NetworkDiagnostic::Client to unify and simplify how otNetworkDiagData arrays (e.g. mNetworkData, mChannelPages) are parsed and populated from read TLVs.

@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 network diagnostic module to streamline TLV handling. It simplifies the process of appending channel pages and unifies the parsing logic for diagnostic data arrays, leading to more concise and maintainable code.

Highlights

  • Simplified ChannelPagesTlv Appending: The process of appending ChannelPagesTlv has been simplified by utilizing a generic Tlv::Append() method, replacing a more verbose manual implementation.
  • Introduced ReadDiagData() Helper: A new ReadDiagData() helper method has been added to NetworkDiagnostic::Client to standardize and unify the parsing of otNetworkDiagData arrays (such as mNetworkData and mChannelPages) from received 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.

Changelog
  • src/core/thread/network_diagnostic.cpp
    • Simplified the appending of ChannelPagesTlv by using a generic Tlv::Append function.
    • Introduced Client::ReadDiagData to centralize the parsing of diagnostic data arrays.
    • Refactored NetworkData and ChannelPages parsing to use the new ReadDiagData helper.
  • src/core/thread/network_diagnostic.hpp
    • Added a DiagData typedef for otNetworkDiagData.
    • Declared the new ReadDiagData static helper method.
  • src/core/thread/network_diagnostic_tlvs.hpp
    • Refactored ChannelPagesTlv from a class to a simple typedef, removing its custom implementation details.
Activity
  • No human activity has been recorded on this pull request yet.
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 simplifies handling of Network Diagnostic TLVs. The ChannelPagesTlv is refactored from a class to a typedef, with its construction now using the standard Tlv::Append<>() method. A new helper, ReadDiagData(), is introduced to unify the parsing of diagnostic data arrays like NetworkData and ChannelPages, reducing code duplication. The changes appear correct and improve code clarity.

@github-actions

github-actions Bot commented Mar 17, 2026

Copy link
Copy Markdown

Merging #12706 into main

name branch text data bss total
ot-cli-ftd 7353a38 482288 860 66540 549688
878a5aa 482288 860 66540 549688
+/- +0 +0 +0 +0
ot-ncp-ftd 7353a38 448212 764 61776 510752
878a5aa 448196 764 61776 510736
+/- -16 +0 +0 -16
ot-cli-mtd 7353a38 374976 764 50972 426712
878a5aa 374960 764 50972 426696
+/- -16 +0 +0 -16
ot-ncp-mtd 7353a38 356140 764 46240 403144
878a5aa 356124 764 46240 403128
+/- -16 +0 +0 -16
ot-cli-ftd-br 7353a38 598448 868 136596 735912
878a5aa 598432 868 136596 735896
+/- -16 +0 +0 -16
ot-rcp 7353a38 63328 568 20804 84700
878a5aa 63328 568 20804 84700
+/- +0 +0 +0 +0
Library files
name branch text data bss total
libopenthread-ftd.a 7353a38 249818 95 40319 290232
878a5aa 249806 95 40319 290220
+/- -12 +0 +0 -12
libopenthread-cli-ftd.a 7353a38 61727 0 8091 69818
878a5aa 61727 0 8091 69818
+/- +0 +0 +0 +0
libopenthread-ncp-ftd.a 7353a38 33581 0 5948 39529
878a5aa 33581 0 5948 39529
+/- +0 +0 +0 +0
libopenthread-mtd.a 7353a38 168550 0 24783 193333
878a5aa 168538 0 24783 193321
+/- -12 +0 +0 -12
libopenthread-cli-mtd.a 7353a38 41436 0 8059 49495
878a5aa 41436 0 8059 49495
+/- +0 +0 +0 +0
libopenthread-ncp-mtd.a 7353a38 25805 0 5948 31753
878a5aa 25805 0 5948 31753
+/- +0 +0 +0 +0
libopenthread-ftd-br.a 7353a38 366713 100 110335 477148
878a5aa 366701 100 110335 477136
+/- -12 +0 +0 -12
libopenthread-cli-ftd-br.a 7353a38 80934 0 8131 89065
878a5aa 80934 0 8131 89065
+/- +0 +0 +0 +0
libopenthread-rcp.a 7353a38 9944 0 5060 15004
878a5aa 9944 0 5060 15004
+/- +0 +0 +0 +0
libopenthread-radio.a 7353a38 19715 0 238 19953
878a5aa 19715 0 238 19953
+/- +0 +0 +0 +0

@codecov

codecov Bot commented Mar 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.58%. Comparing base (7353a38) to head (56ff20c).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/core/thread/network_diagnostic.cpp 14.28% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12706      +/-   ##
==========================================
+ Coverage   75.51%   76.58%   +1.07%     
==========================================
  Files         694      694              
  Lines       96165    93409    -2756     
==========================================
- Hits        72618    71537    -1081     
+ Misses      23547    21872    -1675     
Files with missing lines Coverage Δ
src/core/thread/network_diagnostic.hpp 81.81% <ø> (ø)
src/core/thread/network_diagnostic_tlvs.hpp 69.51% <ø> (-1.42%) ⬇️
src/core/thread/network_diagnostic.cpp 74.37% <14.28%> (-1.92%) ⬇️

... and 227 files with indirect coverage changes

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

@abtink
abtink marked this pull request as ready for review March 17, 2026 05:35
This commit simplifies appending `ChannelPagesTlv` using the standard
`Tlv::Append<>()` with the the array of supported channel pages as
the TLV value.

In addition, a `ReadDiagData()` helper method is introduced in the
`NetworkDiagnostic::Client` to unify and simplify how `otNetworkDiagData`
arrays (e.g. `mNetworkData`, `mChannelPages`) are parsed and populated
from read TLVs.
@abtink
abtink force-pushed the diag/channel-page-tlv branch from d7705ff to 56ff20c Compare March 17, 2026 18:36
@jwhui
jwhui merged commit b28b4a6 into openthread:main Mar 17, 2026
110 of 111 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