Skip to content

[dbus] Add leader takeover and local leader weight API#2372

Open
nand-nor wants to merge 1 commit into
openthread:mainfrom
nand-nor:add/takeover-leader-api
Open

[dbus] Add leader takeover and local leader weight API#2372
nand-nor wants to merge 1 commit into
openthread:mainfrom
nand-nor:add/takeover-leader-api

Conversation

@nand-nor
Copy link
Copy Markdown

Adds DBus APIs to takeover leader role and to set the local leader weight

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 12, 2024

Codecov Report

Attention: Patch coverage is 23.52941% with 13 lines in your changes missing coverage. Please review.

Project coverage is 45.37%. Comparing base (2b41187) to head (3022ab4).
Report is 725 commits behind head on main.

Files Patch % Lines
src/dbus/server/dbus_thread_object_rcp.cpp 23.52% 13 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2372       +/-   ##
===========================================
- Coverage   55.77%   45.37%   -10.41%     
===========================================
  Files          87       94        +7     
  Lines        6890     9931     +3041     
  Branches        0      749      +749     
===========================================
+ Hits         3843     4506      +663     
- Misses       3047     5206     +2159     
- Partials        0      219      +219     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nand-nor
Copy link
Copy Markdown
Author

This depends on if/when this OT PR merges openthread/openthread#9186

auto threadHelper = mHost.GetThreadHelper();
otError error = OT_ERROR_NONE;

SuccessOrExit(error = otThreadBecomeLeader(threadHelper->GetInstance()));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From the API doc, this should only be called when the device is attached. Could you add a check before issuing this call?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added a check for this in 95dfb9b

…e method

to the LocalLeaderWeight property, in order to set the local leader weight
@nand-nor nand-nor force-pushed the add/takeover-leader-api branch from 3022ab4 to 95dfb9b Compare June 3, 2026 20:03
@nand-nor
Copy link
Copy Markdown
Author

nand-nor commented Jun 3, 2026

/gemini review

Copy link
Copy Markdown

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

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 adds support for taking over as the partition leader and setting the local leader weight via DBus. Specifically, it introduces the TakeoverLeader method and updates the LocalLeaderWeight property to be writable. The feedback suggests simplifying the TakeoverLeaderHandler implementation by removing a redundant role check, as otThreadBecomeLeader already performs this validation internally.

Comment on lines +1926 to +1937
void DBusThreadObjectRcp::TakeoverLeaderHandler(DBusRequest &aRequest)
{
auto threadHelper = mHost.GetThreadHelper();
otError error = OT_ERROR_NONE;
otDeviceRole role = otThreadGetDeviceRole(threadHelper->GetInstance());

VerifyOrExit(role != OT_DEVICE_ROLE_DISABLED && role != OT_DEVICE_ROLE_DETACHED, error = OT_ERROR_INVALID_STATE);
SuccessOrExit(error = otThreadBecomeLeader(threadHelper->GetInstance()));

exit:
aRequest.ReplyOtResult(error);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The explicit role check in TakeoverLeaderHandler is redundant because otThreadBecomeLeader already performs this check internally and returns OT_ERROR_INVALID_STATE if the device is disabled or detached. We can simplify this handler by directly forwarding the result of otThreadBecomeLeader to ReplyOtResult, similar to other handlers like DetachHandler.

void DBusThreadObjectRcp::TakeoverLeaderHandler(DBusRequest &aRequest)
{
    auto threadHelper = mHost.GetThreadHelper();

    aRequest.ReplyOtResult(otThreadBecomeLeader(threadHelper->GetInstance()));
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants