Skip to content

[border-agent] simplify forwarding logic to leader - #12114

Merged
jwhui merged 1 commit into
openthread:mainfrom
abtink:ba/forward-leader-enhc
Nov 5, 2025
Merged

[border-agent] simplify forwarding logic to leader#12114
jwhui merged 1 commit into
openthread:mainfrom
abtink:ba/forward-leader-enhc

Conversation

@abtink

@abtink abtink commented Nov 5, 2025

Copy link
Copy Markdown
Member

This commit simplifies the CoAP message forwarding logic within the CoapDtlsSession by removing the mPetition and mSeparate boolean flags from the ForwardContext.

The Uri of the request is now stored directly in ForwardContext and used to determine the logic flow, making the code more explicit and easier to understand.

The ForwardToLeader() is only used with kUriLeaderPetition and kUriLeaderKeepAlive, both of which require a separate non-confirmable response in addition to an immediate CoAP Ack (i.e., as if mSeparate is true).

This change removes the need for intermediate flags and simplifies the implementation of ForwardToLeader(), SendErrorMessage(), and the ForwardContext constructor and ToHeader() method. The CoAP message initialization is now more direct, always using kTypeNonConfirmable for forwarded responses and error messages.

This commit simplifies the CoAP message forwarding logic within the
`CoapDtlsSession` by removing the `mPetition` and `mSeparate` boolean
flags from the `ForwardContext`.

The `Uri` of the request is now stored directly in `ForwardContext`
and used to determine the logic flow, making the code more explicit
and easier to understand.

The `ForwardToLeader()` is only used with `kUriLeaderPetition` and
`kUriLeaderKeepAlive`, both of which requires a separate
non-confirmable response in addition to an immediate CoAP Ack
(i.e., as if `mSeperate` is `true`).

This change removes the need for intermediate flags and simplifies the
implementation of `ForwardToLeader()`, `SendErrorMessage()`, and the
`ForwardContext` constructor and `ToHeader()` method. The CoAP
message initialization is now more direct, always using
`kTypeNonConfirmable` for forwarded responses and error messages.
@github-actions

github-actions Bot commented Nov 5, 2025

Copy link
Copy Markdown

Merging #12114 into main

name branch text data bss total
ot-cli-ftd 8f11e4a 481248 860 66580 548688
ef5a214 481088 860 66580 548528
+/- -160 +0 +0 -160
ot-ncp-ftd 8f11e4a 447188 764 61824 509776
ef5a214 447004 764 61824 509592
+/- -184 +0 +0 -184
ot-cli-mtd 8f11e4a 373200 764 51052 425016
ef5a214 373200 764 51052 425016
+/- +0 +0 +0 +0
ot-ncp-mtd 8f11e4a 354164 764 46320 401248
ef5a214 354164 764 46320 401248
+/- +0 +0 +0 +0
ot-cli-ftd-br 8f11e4a 585232 868 136452 722552
ef5a214 585064 868 136452 722384
+/- -168 +0 +0 -168
ot-rcp 8f11e4a 63200 568 20804 84572
ef5a214 63200 568 20804 84572
+/- +0 +0 +0 +0
Library files
name branch text data bss total
libopenthread-ftd.a 8f11e4a 248161 95 40358 288614
ef5a214 247989 95 40358 288442
+/- -172 +0 +0 -172
libopenthread-cli-ftd.a 8f11e4a 61459 0 8083 69542
ef5a214 61459 0 8083 69542
+/- +0 +0 +0 +0
libopenthread-ncp-ftd.a 8f11e4a 33581 0 5948 39529
ef5a214 33581 0 5948 39529
+/- +0 +0 +0 +0
libopenthread-mtd.a 8f11e4a 166027 0 24854 190881
ef5a214 166027 0 24854 190881
+/- +0 +0 +0 +0
libopenthread-cli-mtd.a 8f11e4a 41414 0 8059 49473
ef5a214 41414 0 8059 49473
+/- +0 +0 +0 +0
libopenthread-ncp-mtd.a 8f11e4a 25805 0 5948 31753
ef5a214 25805 0 5948 31753
+/- +0 +0 +0 +0
libopenthread-ftd-br.a 8f11e4a 354269 100 110198 464567
ef5a214 354097 100 110198 464395
+/- -172 +0 +0 -172
libopenthread-cli-ftd-br.a 8f11e4a 79303 0 8115 87418
ef5a214 79303 0 8115 87418
+/- +0 +0 +0 +0
libopenthread-rcp.a 8f11e4a 9944 0 5060 15004
ef5a214 9944 0 5060 15004
+/- +0 +0 +0 +0
libopenthread-radio.a 8f11e4a 19479 0 238 19717
ef5a214 19479 0 238 19717
+/- +0 +0 +0 +0

@abtink

abtink commented Nov 5, 2025

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 simplifies the CoAP message forwarding logic by removing intermediate flags (mPetition, mSeparate) and using the request URI directly. This is a great refactoring that makes the code more explicit and easier to follow. The changes are consistent and the logic is sound. I have a couple of minor suggestions to further improve conciseness and debuggability.

Comment thread src/core/meshcop/border_agent.cpp
Comment thread src/core/meshcop/border_agent.cpp
@codecov

codecov Bot commented Nov 5, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.44%. Comparing base (8f11e4a) to head (861f056).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/core/meshcop/border_agent.cpp 0.00% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12114      +/-   ##
==========================================
- Coverage   77.77%   69.44%   -8.34%     
==========================================
  Files         668      667       -1     
  Lines       91356   102812   +11456     
==========================================
+ Hits        71056    71399     +343     
- Misses      20300    31413   +11113     
Files with missing lines Coverage Δ
src/core/meshcop/border_agent.hpp 45.00% <ø> (ø)
src/core/meshcop/border_agent.cpp 30.15% <0.00%> (+0.86%) ⬆️

... and 241 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 November 5, 2025 02:38
@jwhui
jwhui requested a review from Copilot November 5, 2025 03:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors the ForwardToLeader functionality in the Border Agent code to simplify the handling of different URI types. The changes remove separate boolean flags (petition and separate) in favor of storing the URI directly, and consolidate response message handling.

Key changes:

  • Replace boolean flags with direct URI storage in ForwardContext
  • Simplify SendErrorMessage to always send non-confirmable error responses
  • Remove conditional ACK logic since all forwarded messages now send ACKs
  • Update response handling to check URI directly instead of using boolean flags

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/core/meshcop/border_agent.hpp Updates ForwardContext structure and SendErrorMessage signature to use URI directly
src/core/meshcop/border_agent.cpp Implements simplified forwarding logic, removes conditional ACK handling, and updates error message generation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/core/meshcop/border_agent.cpp
Comment thread src/core/meshcop/border_agent.cpp
@jwhui
jwhui merged commit 241f6cf into openthread:main Nov 5, 2025
116 of 117 checks passed
@abtink abtink moved this to Done in Enhancing MeshCoP Nov 5, 2025
@abtink abtink assigned abtink and unassigned abtink Nov 5, 2025
wtongze pushed a commit to wtongze/openthread that referenced this pull request Nov 6, 2025
This commit simplifies the CoAP message forwarding logic within the
`CoapDtlsSession` by removing the `mPetition` and `mSeparate` boolean
flags from the `ForwardContext`.

The `Uri` of the request is now stored directly in `ForwardContext`
and used to determine the logic flow, making the code more explicit
and easier to understand.

The `ForwardToLeader()` is only used with `kUriLeaderPetition` and
`kUriLeaderKeepAlive`, both of which requires a separate
non-confirmable response in addition to an immediate CoAP Ack
(i.e., as if `mSeperate` is `true`).

This change removes the need for intermediate flags and simplifies the
implementation of `ForwardToLeader()`, `SendErrorMessage()`, and the
`ForwardContext` constructor and `ToHeader()` method. The CoAP
message initialization is now more direct, always using
`kTypeNonConfirmable` for forwarded responses and error messages.
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.

4 participants