openthread / ot-br-posix

OpenThread Border Router, a Thread border router for POSIX-based platforms.
https://openthread.io/
BSD 3-Clause "New" or "Revised" License
420 stars 235 forks source link

[dbus] add `retain_active_session` option for deactivate API #2537

Closed mia1yang closed 1 month ago

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 45.51%. Comparing base (2b41187) to head (fd41cd1). Report is 834 commits behind head on main.

Files with missing lines Patch % Lines
src/dbus/server/dbus_thread_object_rcp.cpp 0.00% 5 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2537 +/- ## =========================================== - Coverage 55.77% 45.51% -10.26% =========================================== Files 87 102 +15 Lines 6890 12197 +5307 Branches 0 894 +894 =========================================== + Hits 3843 5552 +1709 - Misses 3047 6343 +3296 - Partials 0 302 +302 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bukepo commented 1 month ago

Should disconnect be the default behavior? Perhaps adding an option "retain-active-session"?

wgtdkp commented 1 month ago

Should disconnect be the default behavior? Perhaps adding an option "retain-active-session"?

+1

mia1yang commented 1 month ago

Should disconnect be the default behavior? Perhaps adding an option "retain-active-session"?

Change force disconnect to be the default logic now. Thanks~

bukepo commented 1 month ago

missing test?

mia1yang commented 1 month ago

missing test?

Test is in openthread branch, and it seems no depends-on support on otbr (only ot have depends-on of otbr). Need to merge openthread change firstly to clear the CI failure.

mia1yang commented 1 month ago

The CI failure of this PR is due to Dbus API new option added, but the original unit test in OT has not added the new option. The CI failure should be clear after PR https://github.com/openthread/openthread/pull/10829 merged. Highlight error info here: "ERROR:dbus.connection:Unable to set arguments () according to signature 'b'; TypeError: More items found in D-Bus signature than in Python arguments"