Skip to content

[coap] encapsulate Request state and hide its Metadata - #12675

Merged
jwhui merged 1 commit into
openthread:mainfrom
abtink:coap/hide-request-metdata
Mar 13, 2026
Merged

[coap] encapsulate Request state and hide its Metadata#12675
jwhui merged 1 commit into
openthread:mainfrom
abtink:coap/hide-request-metdata

Conversation

@abtink

@abtink abtink commented Mar 11, 2026

Copy link
Copy Markdown
Member

This commit updates the CoapBase::Request class to encapsulate its internal state. The mMessage pointer and mMetadata struct are now private, and their properties are accessed and modified through explicit getter and setter methods (e.g., GetMessage(), IsConfirmable(), MarkAsAcknowledged()).

By doing so, the code that manages pending requests no longer directly manipulates the internal metadata fields, improving code structure and maintainability.

@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 significantly refactors the CoapBase::Request class to enhance its object-oriented design by encapsulating its internal state. By making the mMessage and mMetadata members private and providing controlled access through public methods, the changes improve code maintainability, reduce direct manipulation of internal data, and promote a cleaner interface for managing CoAP requests.

Highlights

  • Encapsulation of Request State: The internal state of CoapBase::Request, specifically the mMessage pointer and mMetadata struct, was made private to enforce encapsulation.
  • New Accessor Methods: Introduced explicit public getter and setter methods (e.g., GetMessage(), IsConfirmable(), MarkAsAcknowledged()) within CoapBase::Request to control access to its internal properties.
  • Refactored Metadata Logic: Methods previously part of CoapBase::Request::Metadata (like HasSamePeerAddrAndPort, ShouldRetransmit, UpdateRetxCounterAndTimeout, IsObserveSubscription) were moved and promoted to public methods of the CoapBase::Request class.
  • Relocated ProcessObserveSend: The ProcessObserveSend method was moved from the CoapBase class to the CoapBase::PendingRequests nested class, improving logical grouping.

🧠 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 effectively encapsulates the CoapBase::Request state by making its internal members private and introducing getter/setter methods. The changes improve code structure and maintainability as intended. I found one critical issue in src/core/coap/coap.hpp where a method was renamed incorrectly, which will cause a compilation error. Other than that, the refactoring looks good.

Comment thread src/core/coap/coap.hpp Outdated
This commit updates the `CoapBase::Request` class to encapsulate its
internal state. The `mMessage` pointer and `mMetadata` struct are
now private, and their properties are accessed and modified through
explicit getter and setter methods (e.g., `GetMessage()`,
`IsConfirmable()`, `MarkAsAcknowledged()`).

By doing so, the code that manages pending requests no longer directly
manipulates the internal metadata fields, improving code structure and
maintainability.
@abtink
abtink force-pushed the coap/hide-request-metdata branch from 2adef51 to 5ec45de Compare March 11, 2026 23:47
@abtink

abtink commented Mar 11, 2026

Copy link
Copy Markdown
Member Author

/gemini review

@github-actions

Copy link
Copy Markdown

Merging #12675 into main

name branch text data bss total
ot-cli-ftd 7696e38 482192 860 66540 549592
cda4f2d 482208 860 66540 549608
+/- +16 +0 +0 +16
ot-ncp-ftd 7696e38 448124 764 61776 510664
cda4f2d 448124 764 61776 510664
+/- +0 +0 +0 +0
ot-cli-mtd 7696e38 374888 764 50972 426624
cda4f2d 374888 764 50972 426624
+/- +0 +0 +0 +0
ot-ncp-mtd 7696e38 356036 764 46240 403040
cda4f2d 356036 764 46240 403040
+/- +0 +0 +0 +0
ot-cli-ftd-br 7696e38 598344 868 136596 735808
cda4f2d 598344 868 136596 735808
+/- +0 +0 +0 +0
ot-rcp 7696e38 63328 568 20804 84700
cda4f2d 63328 568 20804 84700
+/- +0 +0 +0 +0
Library files
name branch text data bss total
libopenthread-ftd.a 7696e38 249830 95 40319 290244
cda4f2d 249838 95 40319 290252
+/- +8 +0 +0 +8
libopenthread-cli-ftd.a 7696e38 61727 0 8091 69818
cda4f2d 61727 0 8091 69818
+/- +0 +0 +0 +0
libopenthread-ncp-ftd.a 7696e38 33581 0 5948 39529
cda4f2d 33581 0 5948 39529
+/- +0 +0 +0 +0
libopenthread-mtd.a 7696e38 168562 0 24783 193345
cda4f2d 168570 0 24783 193353
+/- +8 +0 +0 +8
libopenthread-cli-mtd.a 7696e38 41436 0 8059 49495
cda4f2d 41436 0 8059 49495
+/- +0 +0 +0 +0
libopenthread-ncp-mtd.a 7696e38 25805 0 5948 31753
cda4f2d 25805 0 5948 31753
+/- +0 +0 +0 +0
libopenthread-ftd-br.a 7696e38 366645 100 110335 477080
cda4f2d 366653 100 110335 477088
+/- +8 +0 +0 +8
libopenthread-cli-ftd-br.a 7696e38 80934 0 8131 89065
cda4f2d 80934 0 8131 89065
+/- +0 +0 +0 +0
libopenthread-rcp.a 7696e38 9944 0 5060 15004
cda4f2d 9944 0 5060 15004
+/- +0 +0 +0 +0
libopenthread-radio.a 7696e38 19715 0 238 19953
cda4f2d 19715 0 238 19953
+/- +0 +0 +0 +0

@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 CoapBase::Request class to improve encapsulation by making its internal state private. The direct access to mMessage and mMetadata is replaced by a set of getter and setter methods. This is a good improvement to the code structure and maintainability. The changes are consistent and correctly implemented across the codebase. The ProcessObserveSend method has also been logically moved to the PendingRequests class. The implementation looks solid and I have no specific comments.

@codecov

codecov Bot commented Mar 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.67089% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.80%. Comparing base (7696e38) to head (5ec45de).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/core/coap/coap.cpp 91.37% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12675      +/-   ##
==========================================
- Coverage   74.58%   73.80%   -0.78%     
==========================================
  Files         695      694       -1     
  Lines       95626    96562     +936     
==========================================
- Hits        71318    71270      -48     
- Misses      24308    25292     +984     
Files with missing lines Coverage Δ
src/core/coap/coap.hpp 96.15% <100.00%> (+2.40%) ⬆️
src/core/coap/coap.cpp 86.95% <91.37%> (+0.17%) ⬆️

... and 125 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 13, 2026 02:47
@github-project-automation github-project-automation Bot moved this to Under review in Improving CoAP/TMF Mar 13, 2026
@jwhui
jwhui merged commit 3d975f7 into openthread:main Mar 13, 2026
111 of 112 checks passed
@github-project-automation github-project-automation Bot moved this from Under review to Merged in Improving CoAP/TMF Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

3 participants