From d052d919185f63882b292f17bb19be9de841d5a1 Mon Sep 17 00:00:00 2001 From: Yakun Xu Date: Tue, 29 Oct 2024 00:24:26 +0800 Subject: [PATCH 01/11] [agent] add mainloop poll timeout config (#2555) --- src/agent/application.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/agent/application.cpp b/src/agent/application.cpp index c656d249a4d..35aa14b5b46 100644 --- a/src/agent/application.cpp +++ b/src/agent/application.cpp @@ -44,8 +44,12 @@ namespace otbr { +#ifndef OTBR_MAINLOOP_POLL_TIMEOUT_SEC +#define OTBR_MAINLOOP_POLL_TIMEOUT_SEC 10 +#endif + std::atomic_bool Application::sShouldTerminate(false); -const struct timeval Application::kPollTimeout = {10, 0}; +const struct timeval Application::kPollTimeout = {OTBR_MAINLOOP_POLL_TIMEOUT_SEC, 0}; Application::Application(const std::string &aInterfaceName, const std::vector &aBackboneInterfaceNames, From 8fe126f7bb6cf5df327c0481009a42ca316e0409 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 28 Oct 2024 09:49:44 -0700 Subject: [PATCH 02/11] submodule: bump third_party/openthread/repo from `6827344` to `7886083` (#2565) Bumps [third_party/openthread/repo](https://github.com/openthread/openthread) from `6827344` to `7886083`. - [Commits](https://github.com/openthread/openthread/compare/6827344e08ea35a130119f2ac103e8d496fd18e0...788608335e95426171f34c0e780d814c76275d30) --- updated-dependencies: - dependency-name: third_party/openthread/repo dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- third_party/openthread/repo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/openthread/repo b/third_party/openthread/repo index 6827344e08e..788608335e9 160000 --- a/third_party/openthread/repo +++ b/third_party/openthread/repo @@ -1 +1 @@ -Subproject commit 6827344e08ea35a130119f2ac103e8d496fd18e0 +Subproject commit 788608335e95426171f34c0e780d814c76275d30 From 7c8e39a53f7b1058da8748bc7f11e753f53b3794 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 29 Oct 2024 07:51:01 -0700 Subject: [PATCH 03/11] submodule: bump third_party/openthread/repo from `7886083` to `005c5ce` (#2568) Bumps [third_party/openthread/repo](https://github.com/openthread/openthread) from `7886083` to `005c5ce`. - [Commits](https://github.com/openthread/openthread/compare/788608335e95426171f34c0e780d814c76275d30...005c5cefc22aaf0396e4327ee7f2e0ad32a7733b) --- updated-dependencies: - dependency-name: third_party/openthread/repo dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- third_party/openthread/repo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/openthread/repo b/third_party/openthread/repo index 788608335e9..005c5cefc22 160000 --- a/third_party/openthread/repo +++ b/third_party/openthread/repo @@ -1 +1 @@ -Subproject commit 788608335e95426171f34c0e780d814c76275d30 +Subproject commit 005c5cefc22aaf0396e4327ee7f2e0ad32a7733b From aa00c768ce7532f1c340d122f72eafdde7b47fe3 Mon Sep 17 00:00:00 2001 From: Li Cao Date: Tue, 29 Oct 2024 22:58:37 +0800 Subject: [PATCH 04/11] [mdns] refactor mDNS State subscription (#2560) This commit refactors the mDNS State subscription using the Subject-Observer pattern. The commit makes the state subscription more flexible. Currently the mDNS state is published in `Application::HandleMdnsState`. Now we have both NCP and RCP case. This function will be difficult to implement. With this new pattern, we can register different observers during NCP/RCP initialization. --- src/agent/application.cpp | 35 ++++++++--------- src/agent/application.hpp | 8 +--- src/border_agent/border_agent.hpp | 4 +- src/mdns/mdns.cpp | 18 +++++++++ src/mdns/mdns.hpp | 58 +++++++++++++++++++++++++++++ src/sdp_proxy/advertising_proxy.hpp | 4 +- src/sdp_proxy/discovery_proxy.hpp | 2 +- src/trel_dnssd/trel_dnssd.hpp | 4 +- 8 files changed, 100 insertions(+), 33 deletions(-) diff --git a/src/agent/application.cpp b/src/agent/application.cpp index 35aa14b5b46..381d60aadf5 100644 --- a/src/agent/application.cpp +++ b/src/agent/application.cpp @@ -70,7 +70,8 @@ Application::Application(const std::string &aInterfaceName, /* aDryRun */ false, aEnableAutoAttach)) #if OTBR_ENABLE_MDNS - , mPublisher(Mdns::Publisher::Create([this](Mdns::Publisher::State aState) { this->HandleMdnsState(aState); })) + , mPublisher( + Mdns::Publisher::Create([this](Mdns::Publisher::State aState) { mMdnsStateSubject.UpdateState(aState); })) #endif #if OTBR_ENABLE_DBUS_SERVER && OTBR_ENABLE_BORDER_AGENT , mDBusAgent(MakeUnique(*mHost, *mPublisher)) @@ -195,24 +196,6 @@ otbrError Application::Run(void) return error; } -void Application::HandleMdnsState(Mdns::Publisher::State aState) -{ - OTBR_UNUSED_VARIABLE(aState); - -#if OTBR_ENABLE_BORDER_AGENT - mBorderAgent->HandleMdnsState(aState); -#endif -#if OTBR_ENABLE_SRP_ADVERTISING_PROXY - mAdvertisingProxy->HandleMdnsState(aState); -#endif -#if OTBR_ENABLE_DNSSD_DISCOVERY_PROXY - mDiscoveryProxy->HandleMdnsState(aState); -#endif -#if OTBR_ENABLE_TREL - mTrelDnssd->HandleMdnsState(aState); -#endif -} - void Application::HandleSignal(int aSignal) { sShouldTerminate = true; @@ -253,6 +236,19 @@ void Application::CreateRcpMode(const std::string &aRestListenAddress, int aRest void Application::InitRcpMode(void) { +#if OTBR_ENABLE_BORDER_AGENT + mMdnsStateSubject.AddObserver(*mBorderAgent); +#endif +#if OTBR_ENABLE_SRP_ADVERTISING_PROXY + mMdnsStateSubject.AddObserver(*mAdvertisingProxy); +#endif +#if OTBR_ENABLE_DNSSD_DISCOVERY_PROXY + mMdnsStateSubject.AddObserver(*mDiscoveryProxy); +#endif +#if OTBR_ENABLE_TREL + mMdnsStateSubject.AddObserver(*mTrelDnssd); +#endif + #if OTBR_ENABLE_MDNS mPublisher->Start(); #endif @@ -300,6 +296,7 @@ void Application::DeinitRcpMode(void) mBorderAgent->SetEnabled(false); #endif #if OTBR_ENABLE_MDNS + mMdnsStateSubject.Clear(); mPublisher->Stop(); #endif } diff --git a/src/agent/application.hpp b/src/agent/application.hpp index 92b44ef1580..c09d6908ff7 100644 --- a/src/agent/application.hpp +++ b/src/agent/application.hpp @@ -237,13 +237,6 @@ class Application : private NonCopyable } #endif - /** - * This method handles mDNS publisher's state changes. - * - * @param[in] aState The state of mDNS publisher. - */ - void HandleMdnsState(Mdns::Publisher::State aState); - private: // Default poll timeout. static const struct timeval kPollTimeout; @@ -264,6 +257,7 @@ class Application : private NonCopyable const char *mBackboneInterfaceName; std::unique_ptr mHost; #if OTBR_ENABLE_MDNS + Mdns::StateSubject mMdnsStateSubject; std::unique_ptr mPublisher; #endif #if OTBR_ENABLE_BORDER_AGENT diff --git a/src/border_agent/border_agent.hpp b/src/border_agent/border_agent.hpp index ed9686f4c3f..ce47e42d5bf 100644 --- a/src/border_agent/border_agent.hpp +++ b/src/border_agent/border_agent.hpp @@ -75,7 +75,7 @@ namespace otbr { /** * This class implements Thread border agent functionality. */ -class BorderAgent : private NonCopyable +class BorderAgent : public Mdns::StateObserver, private NonCopyable { public: /** The callback for receiving ephemeral key changes. */ @@ -139,7 +139,7 @@ class BorderAgent : private NonCopyable * * @param[in] aState The state of mDNS publisher. */ - void HandleMdnsState(Mdns::Publisher::State aState); + void HandleMdnsState(Mdns::Publisher::State aState) override; /** * This method creates ephemeral key in the Border Agent. diff --git a/src/mdns/mdns.cpp b/src/mdns/mdns.cpp index 3774a8b477d..7fd050c42f4 100644 --- a/src/mdns/mdns.cpp +++ b/src/mdns/mdns.cpp @@ -779,6 +779,24 @@ void Publisher::RemoveAddress(AddressList &aAddressList, const Ip6Address &aAddr } } +void StateSubject::AddObserver(StateObserver &aObserver) +{ + mObservers.push_back(&aObserver); +} + +void StateSubject::UpdateState(Publisher::State aState) +{ + for (StateObserver *observer : mObservers) + { + observer->HandleMdnsState(aState); + } +} + +void StateSubject::Clear(void) +{ + mObservers.clear(); +} + } // namespace Mdns } // namespace otbr diff --git a/src/mdns/mdns.hpp b/src/mdns/mdns.hpp index 45dff98d29c..7044d1c9427 100644 --- a/src/mdns/mdns.hpp +++ b/src/mdns/mdns.hpp @@ -665,6 +665,64 @@ class Publisher : private NonCopyable MdnsTelemetryInfo mTelemetryInfo{}; }; +/** + * This interface is a mDNS State Observer. + */ +class StateObserver +{ +public: + /** + * This method notifies the mDNS state to the observer. + * + * @param[in] aState The mDNS State. + */ + virtual void HandleMdnsState(Publisher::State aState) = 0; + + /** + * The destructor. + */ + virtual ~StateObserver(void) = default; +}; + +/** + * This class defines a mDNS State Subject. + */ +class StateSubject +{ +public: + /** + * Constructor. + */ + StateSubject(void) = default; + + /** + * Destructor. + */ + ~StateSubject(void) = default; + + /** + * This method adds an mDNS State Observer to this subject. + * + * @param[in] aObserver A reference to the observer. If it's nullptr, it won't be added. + */ + void AddObserver(StateObserver &aObserver); + + /** + * This method updates the mDNS State. + * + * @param[in] aState The mDNS State. + */ + void UpdateState(Publisher::State aState); + + /** + * This method removes all the observers. + */ + void Clear(void); + +private: + std::vector mObservers; +}; + /** * @} */ diff --git a/src/sdp_proxy/advertising_proxy.hpp b/src/sdp_proxy/advertising_proxy.hpp index 2077d35010e..1fe1ca3ed8a 100644 --- a/src/sdp_proxy/advertising_proxy.hpp +++ b/src/sdp_proxy/advertising_proxy.hpp @@ -52,7 +52,7 @@ namespace otbr { /** * This class implements the Advertising Proxy. */ -class AdvertisingProxy : private NonCopyable +class AdvertisingProxy : public Mdns::StateObserver, private NonCopyable { public: /** @@ -80,7 +80,7 @@ class AdvertisingProxy : private NonCopyable * * @param[in] aState The state of mDNS publisher. */ - void HandleMdnsState(Mdns::Publisher::State aState); + void HandleMdnsState(Mdns::Publisher::State aState) override; private: struct OutstandingUpdate diff --git a/src/sdp_proxy/discovery_proxy.hpp b/src/sdp_proxy/discovery_proxy.hpp index 9278cf49667..ac7a2932ec1 100644 --- a/src/sdp_proxy/discovery_proxy.hpp +++ b/src/sdp_proxy/discovery_proxy.hpp @@ -56,7 +56,7 @@ namespace Dnssd { /** * This class implements the DNS-SD Discovery Proxy. */ -class DiscoveryProxy : private NonCopyable +class DiscoveryProxy : public Mdns::StateObserver, private NonCopyable { public: /** diff --git a/src/trel_dnssd/trel_dnssd.hpp b/src/trel_dnssd/trel_dnssd.hpp index d6856b1d8ae..5c50a3ebd18 100644 --- a/src/trel_dnssd/trel_dnssd.hpp +++ b/src/trel_dnssd/trel_dnssd.hpp @@ -60,7 +60,7 @@ namespace TrelDnssd { * @{ */ -class TrelDnssd +class TrelDnssd : public Mdns::StateObserver { public: /** @@ -107,7 +107,7 @@ class TrelDnssd * * @param[in] aState The state of mDNS publisher. */ - void HandleMdnsState(Mdns::Publisher::State aState); + void HandleMdnsState(Mdns::Publisher::State aState) override; private: static constexpr size_t kPeerCacheSize = 256; From 59f89f01d392d70376fbfc2f051463edb6c7ebe0 Mon Sep 17 00:00:00 2001 From: Li Cao Date: Tue, 29 Oct 2024 23:08:04 +0800 Subject: [PATCH 05/11] [tests] enhance ncp mode test script (#2563) This commit enhances the ncp_mode github workflow in CI. A few enhancement is done: 1. Build docker image in the workflow for testing the border routing functionality. 2. Change the usage of `ncp_mode` script. Use the same parameter style as `script/test`. 3. Accelerate the workflow by avoiding building unnecessary dependencies: openthread library (in bootstrap), unit test, OTBR web --- .github/workflows/ncp_mode.yml | 20 +++++- script/test | 5 +- tests/scripts/ncp_mode | 124 ++++++++++++++++++++++++--------- 3 files changed, 109 insertions(+), 40 deletions(-) diff --git a/.github/workflows/ncp_mode.yml b/.github/workflows/ncp_mode.yml index 0370311ae6d..803642935e9 100644 --- a/.github/workflows/ncp_mode.yml +++ b/.github/workflows/ncp_mode.yml @@ -49,9 +49,11 @@ jobs: matrix: mdns: ["mDNSResponder", "avahi"] env: - BUILD_TARGET: check + BUILD_TARGET: ncp_mode OTBR_MDNS: ${{ matrix.mdns }} OTBR_COVERAGE: 1 + OTBR_VERBOSE: 1 + OTBR_OPTIONS: "-DCMAKE_BUILD_TYPE=Debug -DOT_THREAD_VERSION=1.4 -DOTBR_COVERAGE=ON -DOTBR_DBUS=ON -DOTBR_FEATURE_FLAGS=ON -DOTBR_TELEMETRY_DATA_API=ON -DOTBR_UNSECURE_JOIN=ON -DOTBR_TREL=ON -DBUILD_TESTING=OFF" steps: - uses: actions/checkout@v4 with: @@ -60,8 +62,20 @@ jobs: run: tests/scripts/bootstrap.sh - name: Build run: | - OTBR_BUILD_DIR="./build/temp" script/cmake-build -DCMAKE_BUILD_TYPE=Debug -DOT_THREAD_VERSION=1.4 -DOTBR_COVERAGE=ON -DOTBR_DBUS=ON -DOTBR_FEATURE_FLAGS=ON -DOTBR_TELEMETRY_DATA_API=ON -DOTBR_WEB=ON -DOTBR_UNSECURE_JOIN=ON -DOTBR_TREL=ON + OTBR_BUILD_DIR="./build/temp" script/cmake-build ${OTBR_OPTIONS} + - name: Build OTBR Docker Image + run: | + sudo docker build -t otbr-ncp \ + -f ./etc/docker/Dockerfile . \ + --build-arg NAT64=0 \ + --build-arg NAT64_SERVICE=0 \ + --build-arg DNS64=0 \ + --build-arg WEB_GUI=0 \ + --build-arg REST_API=0 \ + --build-arg FIREWALL=0 \ + --build-arg OTBR_OPTIONS="${OTBR_OPTIONS}" - name: Run - run: OTBR_VERBOSE=1 OTBR_TOP_BUILDDIR="./build/temp" script/test ncp_mode + run: | + top_builddir="./build/temp" tests/scripts/ncp_mode build_ot_sim expect - name: Codecov uses: codecov/codecov-action@v4 diff --git a/script/test b/script/test index ca9398e0452..7914d23564c 100755 --- a/script/test +++ b/script/test @@ -232,14 +232,11 @@ main() do_doxygen ;; help) - print_usage + print_usage 1 ;; meshcop) top_builddir="${OTBR_TOP_BUILDDIR}" print_result ./tests/scripts/meshcop ;; - ncp_mode) - top_builddir="${OTBR_TOP_BUILDDIR}" print_result ./tests/scripts/ncp_mode - ;; openwrt) print_result ./tests/scripts/openwrt ;; diff --git a/tests/scripts/ncp_mode b/tests/scripts/ncp_mode index a6d00067273..8382d904bce 100755 --- a/tests/scripts/ncp_mode +++ b/tests/scripts/ncp_mode @@ -46,6 +46,9 @@ readonly OT_CLI OT_NCP="${OT_NCP:-ot-ncp-ftd}" readonly OT_NCP +OTBR_DOCKER_IMAGE="${OTBR_DOCKER_IMAGE:-otbr-ncp}" +readonly OTBR_DOCKER_IMAGE + ABS_TOP_BUILDDIR="$(cd "${top_builddir:-"${SCRIPT_DIR}"/../../}" && pwd)" readonly ABS_TOP_BUILDDIR @@ -126,12 +129,39 @@ readonly TUN_NAME #---------------------------------------- # Test steps #---------------------------------------- -build_ot_simulation() +do_build_ot_simulation() { sudo rm -rf "${ABS_TOP_OT_BUILDDIR}/ncp" sudo rm -rf "${ABS_TOP_OT_BUILDDIR}/cli" - OT_CMAKE_BUILD_DIR=${ABS_TOP_OT_BUILDDIR}/ncp "${ABS_TOP_OT_SRCDIR}"/script/cmake-build simulation -DOT_MTD=OFF -DOT_APP_CLI=OFF -DOT_APP_RCP=OFF - OT_CMAKE_BUILD_DIR=${ABS_TOP_OT_BUILDDIR}/cli "${ABS_TOP_OT_SRCDIR}"/script/cmake-build simulation -DOT_MTD=OFF -DOT_APP_NCP=OFF -DOT_APP_RCP=OFF -DOT_RCP=OFF + OT_CMAKE_BUILD_DIR=${ABS_TOP_OT_BUILDDIR}/ncp "${ABS_TOP_OT_SRCDIR}"/script/cmake-build simulation \ + -DOT_MTD=OFF -DOT_RCP=OFF -DOT_APP_CLI=OFF -DOT_APP_RCP=OFF \ + -DOT_BORDER_ROUTING=ON -DOT_NCP_INFRA_IF=ON -DOT_SIMULATION_INFRA_IF=OFF \ + -DBUILD_TESTING=OFF + OT_CMAKE_BUILD_DIR=${ABS_TOP_OT_BUILDDIR}/cli "${ABS_TOP_OT_SRCDIR}"/script/cmake-build simulation \ + -DOT_MTD=OFF -DOT_RCP=OFF -DOT_APP_NCP=OFF -DOT_APP_RCP=OFF \ + -DOT_BORDER_ROUTING=OFF \ + -DBUILD_TESTING=OFF +} + +do_build_otbr_docker() +{ + otbr_docker_options=( + "-DOT_THREAD_VERSION=1.4" + "-DOTBR_DBUS=ON" + "-DOTBR_FEATURE_FLAGS=ON" + "-DOTBR_TELEMETRY_DATA_API=ON" + "-DOTBR_TREL=ON" + "-DOTBR_LINK_METRICS_TELEMETRY=ON" + ) + sudo docker build -t "${OTBR_DOCKER_IMAGE}" \ + -f ./etc/docker/Dockerfile . \ + --build-arg NAT64=0 \ + --build-arg NAT64_SERVICE=0 \ + --build-arg DNS64=0 \ + --build-arg WEB_GUI=0 \ + --build-arg REST_API=0 \ + --build-arg FIREWALL=0 \ + --build-arg OTBR_OPTIONS="${otbr_docker_options[*]}" } test_setup() @@ -143,9 +173,14 @@ test_setup() # OPENTHREAD_POSIX_DAEMON_SOCKET_LOCK sudo rm -vf "/tmp/openthread.lock" - [[ ${BUILD_OT_SIM} == 1 ]] && build_ot_simulation ot_cli=$(find "${ABS_TOP_OT_BUILDDIR}" -name "${OT_CLI}") ot_ncp=$(find "${ABS_TOP_OT_BUILDDIR}" -name "${OT_NCP}") + executable_or_die "${ot_cli}" + executable_or_die "${ot_ncp}" + + export EXP_OTBR_AGENT_PATH="${OTBR_AGENT_PATH}" + export EXP_OT_CLI_PATH="${ot_cli}" + export EXP_OT_NCP_PATH="${ot_ncp}" # We will be creating a lot of log information # Rotate logs so we have a clean and empty set of logs uncluttered with other stuff @@ -217,45 +252,68 @@ otbr_exec_expect_script() done } -parse_args() +do_expect() { - BUILD_OT_SIM=1 - RUN_ALL_TESTS=1 + if [[ $# != 0 ]]; then + otbr_exec_expect_script "$@" + else + mapfile -t test_files < <(find "${EXPECT_SCRIPT_DIR}" -type f -name "ncp_*.exp") + otbr_exec_expect_script "${test_files[@]}" || die "ncp expect script failed!" + fi - while [[ $# -gt 0 ]]; do - case $1 in - --build-ot-sim) - BUILD_OT_SIM="$2" - shift - ;; - --one-test) - RUN_ALL_TESTS=0 - TEST_NAME="$2" - shift - ;; - esac - shift - done + exit 0 } -main() +print_usage() { - parse_args "$@" + cat < Date: Tue, 29 Oct 2024 13:15:41 -0400 Subject: [PATCH 06/11] [border-agent] improve the documentation returned by --help (#2444) Options that had both short and long names are rendered as such to make scripts easier to read. Options that were undocumented were added. --- src/agent/main.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/agent/main.cpp b/src/agent/main.cpp index e47c96439e5..525ea3f74b5 100644 --- a/src/agent/main.cpp +++ b/src/agent/main.cpp @@ -60,10 +60,12 @@ #endif #endif -static const char kDefaultInterfaceName[] = "wpan0"; +#define DEFAULT_INTERFACE_NAME "wpan0" +static const char kDefaultInterfaceName[] = DEFAULT_INTERFACE_NAME; // Port number used by Rest server. static const uint32_t kPortNumber = 8081; +#define HELP_DEFAULT_REST_PORT_NUMBER "8081" enum { @@ -144,8 +146,20 @@ static void PrintHelp(const char *aProgramName) fprintf(stderr, "Usage: %s [-I interfaceName] [-B backboneIfName] [-d DEBUG_LEVEL] [-v] [-s] [--auto-attach[=0/1]] " "RADIO_URL [RADIO_URL]\n" - " --auto-attach defaults to 1\n" - " -s disables syslog and prints to standard out\n", + " -I, --thread-ifname Name of the Thread network interface (default: " DEFAULT_INTERFACE_NAME ").\n" + " -B, --backbone-ifname Name of the backbone network interfaces (can be specified multiple times).\n" + " -d, --debug-level The log level (EMERG=0, ALERT=1, CRIT=2, ERR=3, WARNING=4, NOTICE=5, INFO=6, " + "DEBUG=7).\n" + " -v, --verbose Enable verbose logging.\n" + " -s, --syslog-disable Disable syslog and print to standard out.\n" + " -h, --help Show this help text.\n" + " -V, --version Print the application's version and exit.\n" + " --radio-version Print the radio coprocessor version and exit.\n" + " --auto-attach Whether or not to automatically attach to the saved network (default: 1).\n" + " --rest-listen-address Network address to listen on for the REST API (default: [::]).\n" + " --rest-listen-port Network port to listen on for the REST API " + "(default: " HELP_DEFAULT_REST_PORT_NUMBER ").\n" + "\n", aProgramName); fprintf(stderr, "%s", otSysGetRadioUrlHelpString()); } From 7e03f9a7293ca90d7fc3b4533612d12cf5b1c693 Mon Sep 17 00:00:00 2001 From: Li Cao Date: Wed, 30 Oct 2024 09:50:29 +0800 Subject: [PATCH 07/11] [tests] add unit test for RcpHost SetCountryCode (#2564) This commit adds a unit test for `RcpHost::SetCountryCode`. The commit also fixes an issue of current usage of `FakePlatform`. In each gtest case, we should get a new OT Instance. Otherwise the OT instance used in last gtest case will have some remaining tasks which are invalid to process. --- tests/gtest/fake_posix_platform.cpp | 17 +++++++--- tests/gtest/test_rcp_host_api.cpp | 52 +++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/tests/gtest/fake_posix_platform.cpp b/tests/gtest/fake_posix_platform.cpp index b806fc3e2f0..96d382409ba 100644 --- a/tests/gtest/fake_posix_platform.cpp +++ b/tests/gtest/fake_posix_platform.cpp @@ -28,10 +28,12 @@ #include "fake_platform.hpp" +#include + #include "openthread/openthread-system.h" -otPlatResetReason gPlatResetReason = OT_PLAT_RESET_REASON_POWER_ON; -static ot::FakePlatform sFakePlatform; +otPlatResetReason gPlatResetReason = OT_PLAT_RESET_REASON_POWER_ON; +static ot::FakePlatform *sFakePlatform; const otRadioSpinelMetrics *otSysGetRadioSpinelMetrics(void) { @@ -60,11 +62,18 @@ otInstance *otSysInit(otPlatformConfig *aPlatformConfig) { OT_UNUSED_VARIABLE(aPlatformConfig); - return sFakePlatform.CurrentInstance(); + assert(sFakePlatform == nullptr); + sFakePlatform = new ot::FakePlatform(); + return sFakePlatform->CurrentInstance(); } void otSysDeinit(void) { + if (sFakePlatform != nullptr) + { + delete sFakePlatform; + sFakePlatform = nullptr; + } } void otSysMainloopUpdate(otInstance *, otSysMainloopContext *) @@ -73,5 +82,5 @@ void otSysMainloopUpdate(otInstance *, otSysMainloopContext *) void otSysMainloopProcess(otInstance *, const otSysMainloopContext *) { - sFakePlatform.Run(/* microseconds */ 1000); + sFakePlatform->Run(/* microseconds */ 1000); } diff --git a/tests/gtest/test_rcp_host_api.cpp b/tests/gtest/test_rcp_host_api.cpp index 0d23e993fbb..9e78b77f5ad 100644 --- a/tests/gtest/test_rcp_host_api.cpp +++ b/tests/gtest/test_rcp_host_api.cpp @@ -129,3 +129,55 @@ TEST(RcpHostApi, DeviceRoleChangesCorrectlyAfterSetThreadEnabled) host.Deinit(); } + +TEST(RcpHostApi, SetCountryCodeWorkCorrectly) +{ + otError error = OT_ERROR_FAILED; + bool resultReceived = false; + otbr::MainloopContext mainloop; + otbr::Ncp::ThreadHost::AsyncResultReceiver receiver = [&resultReceived, &error](otError aError, + const std::string &aErrorMsg) { + OT_UNUSED_VARIABLE(aErrorMsg); + resultReceived = true; + error = aError; + }; + otbr::Ncp::RcpHost host("wpan0", std::vector(), /* aBackboneInterfaceName */ "", /* aDryRun */ false, + /* aEnableAutoAttach */ false); + + // 1. Call SetCountryCode when host hasn't been initialized. + otbr::MainloopManager::GetInstance().RemoveMainloopProcessor( + &host); // Temporarily remove RcpHost because it's not initialized yet. + host.SetCountryCode("AF", receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_INVALID_STATE); + otbr::MainloopManager::GetInstance().AddMainloopProcessor(&host); + + host.Init(); + // 2. Call SetCountryCode with invalid arguments + resultReceived = false; + error = OT_ERROR_NONE; + host.SetCountryCode("AFA", receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_INVALID_ARGS); + + resultReceived = false; + error = OT_ERROR_NONE; + host.SetCountryCode("A", receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_INVALID_ARGS); + + resultReceived = false; + error = OT_ERROR_NONE; + host.SetCountryCode("12", receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_INVALID_ARGS); + + // 3. Call SetCountryCode with valid argument + resultReceived = false; + error = OT_ERROR_NONE; + host.SetCountryCode("AF", receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_NOT_IMPLEMENTED); // The current platform weak implmentation returns 'NOT_IMPLEMENTED'. + + host.Deinit(); +} From f9b3bd321528b896e1a6282f1d5a88b9be8c0737 Mon Sep 17 00:00:00 2001 From: Li Cao Date: Wed, 30 Oct 2024 22:54:55 +0800 Subject: [PATCH 08/11] [thread-host] add set channel max power api (#2551) This commit adds the SetChannelMaxPower method to ThreadHost. The commit implements this method for RcpHost. The implementation for NCP will be added later. --- src/ncp/ncp_host.cpp | 9 +++++++++ src/ncp/ncp_host.hpp | 2 ++ src/ncp/rcp_host.cpp | 29 +++++++++++++++++++++++++++++ src/ncp/rcp_host.hpp | 2 ++ src/ncp/thread_host.hpp | 19 +++++++++++++++++++ 5 files changed, 61 insertions(+) diff --git a/src/ncp/ncp_host.cpp b/src/ncp/ncp_host.cpp index ddefe8581bb..4a37a2740e5 100644 --- a/src/ncp/ncp_host.cpp +++ b/src/ncp/ncp_host.cpp @@ -182,6 +182,15 @@ void NcpHost::GetChannelMasks(const ChannelMasksReceiver &aReceiver, const Async mTaskRunner.Post([aErrReceiver](void) { aErrReceiver(OT_ERROR_NOT_IMPLEMENTED, "Not implemented!"); }); } +void NcpHost::SetChannelMaxPowers(const std::vector &aChannelMaxPowers, + const AsyncResultReceiver &aReceiver) +{ + OT_UNUSED_VARIABLE(aChannelMaxPowers); + + // TODO: Implement SetChannelMaxPowers under NCP mode. + mTaskRunner.Post([aReceiver](void) { aReceiver(OT_ERROR_NOT_IMPLEMENTED, "Not implemented!"); }); +} + void NcpHost::Process(const MainloopContext &aMainloop) { mSpinelDriver.Process(&aMainloop); diff --git a/src/ncp/ncp_host.hpp b/src/ncp/ncp_host.hpp index 16f21669276..d5482a42dee 100644 --- a/src/ncp/ncp_host.hpp +++ b/src/ncp/ncp_host.hpp @@ -93,6 +93,8 @@ class NcpHost : public MainloopProcessor, public ThreadHost, public NcpNetworkPr void SetThreadEnabled(bool aEnabled, const AsyncResultReceiver aReceiver) override; void SetCountryCode(const std::string &aCountryCode, const AsyncResultReceiver &aReceiver) override; void GetChannelMasks(const ChannelMasksReceiver &aReceiver, const AsyncResultReceiver &aErrReceiver) override; + void SetChannelMaxPowers(const std::vector &aChannelMaxPowers, + const AsyncResultReceiver &aReceiver) override; CoprocessorType GetCoprocessorType(void) override { return OT_COPROCESSOR_NCP; } const char *GetCoprocessorVersion(void) override; const char *GetInterfaceName(void) const override { return mConfig.mInterfaceName; } diff --git a/src/ncp/rcp_host.cpp b/src/ncp/rcp_host.cpp index 0ff0cf94e04..4b3e5951ecf 100644 --- a/src/ncp/rcp_host.cpp +++ b/src/ncp/rcp_host.cpp @@ -31,6 +31,7 @@ #include "ncp/rcp_host.hpp" #include +#include #include #include @@ -487,6 +488,34 @@ void RcpHost::GetChannelMasks(const ChannelMasksReceiver &aReceiver, const Async } } +void RcpHost::SetChannelMaxPowers(const std::vector &aChannelMaxPowers, + const AsyncResultReceiver &aReceiver) +{ + otError error = OT_ERROR_NONE; + std::string errorMsg; + + VerifyOrExit(mInstance != nullptr, error = OT_ERROR_INVALID_STATE, errorMsg = "OT is not initialized"); + + for (ChannelMaxPower channelMaxPower : aChannelMaxPowers) + { + VerifyOrExit((channelMaxPower.mChannel >= OT_RADIO_2P4GHZ_OQPSK_CHANNEL_MIN) && + (channelMaxPower.mChannel <= OT_RADIO_2P4GHZ_OQPSK_CHANNEL_MAX), + error = OT_ERROR_INVALID_ARGS, errorMsg = "The channel is invalid"); + } + + for (ChannelMaxPower channelMaxPower : aChannelMaxPowers) + { + otbrLogInfo("Set channel max power: channel=%u, maxPower=%u", static_cast(channelMaxPower.mChannel), + static_cast(channelMaxPower.mMaxPower)); + SuccessOrExit(error = otPlatRadioSetChannelTargetPower( + mInstance, static_cast(channelMaxPower.mChannel), channelMaxPower.mMaxPower), + errorMsg = "Failed to set channel max power"); + } + +exit: + mTaskRunner.Post([aReceiver, error, errorMsg](void) { aReceiver(error, errorMsg); }); +} + void RcpHost::DisableThreadAfterDetach(void *aContext) { static_cast(aContext)->DisableThreadAfterDetach(); diff --git a/src/ncp/rcp_host.hpp b/src/ncp/rcp_host.hpp index 6de94667384..6f216310602 100644 --- a/src/ncp/rcp_host.hpp +++ b/src/ncp/rcp_host.hpp @@ -211,6 +211,8 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro void SetThreadEnabled(bool aEnabled, const AsyncResultReceiver aReceiver) override; void SetCountryCode(const std::string &aCountryCode, const AsyncResultReceiver &aReceiver) override; void GetChannelMasks(const ChannelMasksReceiver &aReceiver, const AsyncResultReceiver &aErrReceiver) override; + void SetChannelMaxPowers(const std::vector &aChannelMaxPowers, + const AsyncResultReceiver &aReceiver) override; CoprocessorType GetCoprocessorType(void) override { diff --git a/src/ncp/thread_host.hpp b/src/ncp/thread_host.hpp index dbc2018a2e3..c4f3f55d913 100644 --- a/src/ncp/thread_host.hpp +++ b/src/ncp/thread_host.hpp @@ -90,6 +90,12 @@ class ThreadHost : virtual public NetworkProperties std::function; using DeviceRoleHandler = std::function; + struct ChannelMaxPower + { + uint16_t mChannel; + int16_t mMaxPower; // INT16_MAX indicates that the corresponding channel is disabled. + }; + /** * Create a Thread Controller Instance. * @@ -182,6 +188,19 @@ class ThreadHost : virtual public NetworkProperties */ virtual void GetChannelMasks(const ChannelMasksReceiver &aReceiver, const AsyncResultReceiver &aErrReceiver) = 0; + /** + * Sets the max power of each channel. + * + * 1. If the host hasn't been initialized, @p aReceiver will be invoked with error OT_ERROR_INVALID_STATE. + * 2. If any value in @p aChannelMaxPowers is invalid, @p aReceiver will be invoked with error + * OT_ERROR_INVALID_ARGS. + * + * @param[in] aChannelMaxPowers A vector of ChannelMaxPower. + * @param[in] aReceiver A receiver to get the async result of this operation. + */ + virtual void SetChannelMaxPowers(const std::vector &aChannelMaxPowers, + const AsyncResultReceiver &aReceiver) = 0; + /** * Returns the co-processor type. */ From 7d6ccd11d014046f6f37de17804838a97ae436c2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 31 Oct 2024 08:59:40 -0700 Subject: [PATCH 09/11] submodule: bump third_party/openthread/repo from `005c5ce` to `ee6fbba` (#2575) Bumps [third_party/openthread/repo](https://github.com/openthread/openthread) from `005c5ce` to `ee6fbba`. - [Commits](https://github.com/openthread/openthread/compare/005c5cefc22aaf0396e4327ee7f2e0ad32a7733b...ee6fbbae05aa127568eef0692290d40a2259a491) --- updated-dependencies: - dependency-name: third_party/openthread/repo dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- third_party/openthread/repo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/openthread/repo b/third_party/openthread/repo index 005c5cefc22..ee6fbbae05a 160000 --- a/third_party/openthread/repo +++ b/third_party/openthread/repo @@ -1 +1 @@ -Subproject commit 005c5cefc22aaf0396e4327ee7f2e0ad32a7733b +Subproject commit ee6fbbae05aa127568eef0692290d40a2259a491 From 62395a5ba2ea6b86ea051f858efeaf98df1584af Mon Sep 17 00:00:00 2001 From: Yakun Xu Date: Fri, 1 Nov 2024 12:36:28 +0800 Subject: [PATCH 10/11] [ncp] pass necessary encoder to encoding func (#2576) Only the encoder is needed for the encoding functions. This commit avoids passing unnecessary context to them. --- src/ncp/ncp_spinel.cpp | 25 +++++++++++++++---------- src/ncp/ncp_spinel.hpp | 2 +- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/ncp/ncp_spinel.cpp b/src/ncp/ncp_spinel.cpp index 586cdd81fc1..12358c05737 100644 --- a/src/ncp/ncp_spinel.cpp +++ b/src/ncp/ncp_spinel.cpp @@ -42,6 +42,7 @@ #include "lib/spinel/spinel.h" #include "lib/spinel/spinel_decoder.hpp" #include "lib/spinel/spinel_driver.hpp" +#include "lib/spinel/spinel_encoder.hpp" #include "lib/spinel/spinel_helper.hpp" namespace otbr { @@ -96,8 +97,8 @@ otbrError NcpSpinel::SpinelDataUnpack(const uint8_t *aDataIn, spinel_size_t aDat void NcpSpinel::DatasetSetActiveTlvs(const otOperationalDatasetTlvs &aActiveOpDatasetTlvs, AsyncTaskPtr aAsyncTask) { otError error = OT_ERROR_NONE; - EncodingFunc encodingFunc = [this, &aActiveOpDatasetTlvs] { - return mEncoder.WriteData(aActiveOpDatasetTlvs.mTlvs, aActiveOpDatasetTlvs.mLength); + EncodingFunc encodingFunc = [&aActiveOpDatasetTlvs](ot::Spinel::Encoder &aEncoder) { + return aEncoder.WriteData(aActiveOpDatasetTlvs.mTlvs, aActiveOpDatasetTlvs.mLength); }; VerifyOrExit(mDatasetSetActiveTask == nullptr, error = OT_ERROR_BUSY); @@ -116,8 +117,8 @@ void NcpSpinel::DatasetMgmtSetPending(std::shared_ptr AsyncTaskPtr aAsyncTask) { otError error = OT_ERROR_NONE; - EncodingFunc encodingFunc = [this, aPendingOpDatasetTlvsPtr] { - return mEncoder.WriteData(aPendingOpDatasetTlvsPtr->mTlvs, aPendingOpDatasetTlvsPtr->mLength); + EncodingFunc encodingFunc = [aPendingOpDatasetTlvsPtr](ot::Spinel::Encoder &aEncoder) { + return aEncoder.WriteData(aPendingOpDatasetTlvsPtr->mTlvs, aPendingOpDatasetTlvsPtr->mLength); }; VerifyOrExit(mDatasetMgmtSetPendingTask == nullptr, error = OT_ERROR_BUSY); @@ -135,7 +136,7 @@ void NcpSpinel::DatasetMgmtSetPending(std::shared_ptr void NcpSpinel::Ip6SetEnabled(bool aEnable, AsyncTaskPtr aAsyncTask) { otError error = OT_ERROR_NONE; - EncodingFunc encodingFunc = [this, aEnable] { return mEncoder.WriteBool(aEnable); }; + EncodingFunc encodingFunc = [aEnable](ot::Spinel::Encoder &aEncoder) { return aEncoder.WriteBool(aEnable); }; VerifyOrExit(mIp6SetEnabledTask == nullptr, error = OT_ERROR_BUSY); @@ -154,7 +155,9 @@ void NcpSpinel::Ip6SetEnabled(bool aEnable, AsyncTaskPtr aAsyncTask) otbrError NcpSpinel::Ip6Send(const uint8_t *aData, uint16_t aLength) { otbrError error = OTBR_ERROR_NONE; - EncodingFunc encodingFunc = [this, aData, aLength] { return mEncoder.WriteDataWithLen(aData, aLength); }; + EncodingFunc encodingFunc = [aData, aLength](ot::Spinel::Encoder &aEncoder) { + return aEncoder.WriteDataWithLen(aData, aLength); + }; SuccessOrExit(SetProperty(SPINEL_PROP_STREAM_NET, encodingFunc), error = OTBR_ERROR_OPENTHREAD); @@ -165,7 +168,7 @@ otbrError NcpSpinel::Ip6Send(const uint8_t *aData, uint16_t aLength) void NcpSpinel::ThreadSetEnabled(bool aEnable, AsyncTaskPtr aAsyncTask) { otError error = OT_ERROR_NONE; - EncodingFunc encodingFunc = [this, aEnable] { return mEncoder.WriteBool(aEnable); }; + EncodingFunc encodingFunc = [aEnable](ot::Spinel::Encoder &aEncoder) { return aEncoder.WriteBool(aEnable); }; VerifyOrExit(mThreadSetEnabledTask == nullptr, error = OT_ERROR_BUSY); @@ -184,7 +187,7 @@ void NcpSpinel::ThreadSetEnabled(bool aEnable, AsyncTaskPtr aAsyncTask) void NcpSpinel::ThreadDetachGracefully(AsyncTaskPtr aAsyncTask) { otError error = OT_ERROR_NONE; - EncodingFunc encodingFunc = [] { return OT_ERROR_NONE; }; + EncodingFunc encodingFunc = [](ot::Spinel::Encoder &) { return OT_ERROR_NONE; }; VerifyOrExit(mThreadDetachGracefullyTask == nullptr, error = OT_ERROR_BUSY); @@ -556,7 +559,9 @@ otbrError NcpSpinel::HandleResponseForPropRemove(spinel_tid_t aTid, otbrError NcpSpinel::Ip6MulAddrUpdateSubscription(const otIp6Address &aAddress, bool aIsAdded) { otbrError error = OTBR_ERROR_NONE; - EncodingFunc encodingFunc = [this, aAddress] { return mEncoder.WriteIp6Address(aAddress); }; + EncodingFunc encodingFunc = [&aAddress](ot::Spinel::Encoder &aEncoder) { + return aEncoder.WriteIp6Address(aAddress); + }; if (aIsAdded) { @@ -613,7 +618,7 @@ otError NcpSpinel::SendCommand(spinel_command_t aCmd, spinel_prop_key_t aKey, co VerifyOrExit(tid != 0, error = OT_ERROR_BUSY); SuccessOrExit(error = mEncoder.BeginFrame(header, aCmd, aKey)); - SuccessOrExit(error = aEncodingFunc()); + SuccessOrExit(error = aEncodingFunc(mEncoder)); SuccessOrExit(error = mEncoder.EndFrame()); SuccessOrExit(error = SendEncodedFrame()); diff --git a/src/ncp/ncp_spinel.hpp b/src/ncp/ncp_spinel.hpp index 8fd9481fee0..fc8ec9275ed 100644 --- a/src/ncp/ncp_spinel.hpp +++ b/src/ncp/ncp_spinel.hpp @@ -290,7 +290,7 @@ class NcpSpinel : public Netif::Dependencies spinel_tid_t GetNextTid(void); void FreeTidTableItem(spinel_tid_t aTid); - using EncodingFunc = std::function; + using EncodingFunc = std::function; otError SendCommand(spinel_command_t aCmd, spinel_prop_key_t aKey, const EncodingFunc &aEncodingFunc); otError SetProperty(spinel_prop_key_t aKey, const EncodingFunc &aEncodingFunc); otError InsertProperty(spinel_prop_key_t aKey, const EncodingFunc &aEncodingFunc); From 5c665075126913d3888bd48aae8265e55dd1417f Mon Sep 17 00:00:00 2001 From: Li Cao Date: Fri, 1 Nov 2024 23:00:27 +0800 Subject: [PATCH 11/11] [thread-host] implement schedule migration for rcp host (#2559) --- src/ncp/rcp_host.cpp | 49 ++++++++++++++++++++++++++--- src/ncp/rcp_host.hpp | 5 +++ tests/gtest/test_rcp_host_api.cpp | 52 +++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 4 deletions(-) diff --git a/src/ncp/rcp_host.cpp b/src/ncp/rcp_host.cpp index 4b3e5951ecf..691c68535f5 100644 --- a/src/ncp/rcp_host.cpp +++ b/src/ncp/rcp_host.cpp @@ -308,7 +308,8 @@ void RcpHost::Deinit(void) mThreadStateChangedCallbacks.clear(); mResetHandlers.clear(); - mSetThreadEnabledReceiver = nullptr; + mSetThreadEnabledReceiver = nullptr; + mScheduleMigrationReceiver = nullptr; } void RcpHost::HandleStateChanged(otChangedFlags aFlags) @@ -425,10 +426,43 @@ void RcpHost::Leave(const AsyncResultReceiver &aReceiver) void RcpHost::ScheduleMigration(const otOperationalDatasetTlvs &aPendingOpDatasetTlvs, const AsyncResultReceiver aReceiver) { - OT_UNUSED_VARIABLE(aPendingOpDatasetTlvs); + otError error = OT_ERROR_NONE; + std::string errorMsg; + otOperationalDataset emptyDataset; - // TODO: Implement ScheduleMigration under RCP mode. - mTaskRunner.Post([aReceiver](void) { aReceiver(OT_ERROR_NOT_IMPLEMENTED, "Not implemented!"); }); + VerifyOrExit(mInstance != nullptr, error = OT_ERROR_INVALID_STATE, errorMsg = "OT is not initialized"); + VerifyOrExit(IsAttached(), error = OT_ERROR_FAILED, + errorMsg = "Cannot schedule migration when this device is detached"); + + // TODO: check supported channel mask + + SuccessOrExit(error = otDatasetSendMgmtPendingSet(mInstance, &emptyDataset, aPendingOpDatasetTlvs.mTlvs, + static_cast(aPendingOpDatasetTlvs.mLength), + SendMgmtPendingSetCallback, this), + errorMsg = "Failed to send MGMT_PENDING_SET.req"); + +exit: + if (error != OT_ERROR_NONE) + { + mTaskRunner.Post([aReceiver, error, errorMsg](void) { aReceiver(error, errorMsg); }); + } + else + { + // otDatasetSendMgmtPendingSet() returns OT_ERROR_BUSY if it has already been called before but the + // callback hasn't been invoked. So we can guarantee that mMigrationReceiver is always nullptr here + assert(mScheduleMigrationReceiver == nullptr); + mScheduleMigrationReceiver = aReceiver; + } +} + +void RcpHost::SendMgmtPendingSetCallback(otError aError, void *aContext) +{ + static_cast(aContext)->SendMgmtPendingSetCallback(aError); +} + +void RcpHost::SendMgmtPendingSetCallback(otError aError) +{ + SafeInvokeAndClear(mScheduleMigrationReceiver, aError, ""); } void RcpHost::SetThreadEnabled(bool aEnabled, const AsyncResultReceiver aReceiver) @@ -553,6 +587,13 @@ void RcpHost::SetCountryCode(const std::string &aCountryCode, const AsyncResultR mTaskRunner.Post([aReceiver, error, errorMsg](void) { aReceiver(error, errorMsg); }); } +bool RcpHost::IsAttached(void) +{ + otDeviceRole role = GetDeviceRole(); + + return role == OT_DEVICE_ROLE_CHILD || role == OT_DEVICE_ROLE_ROUTER || role == OT_DEVICE_ROLE_LEADER; +} + /* * Provide, if required an "otPlatLog()" function */ diff --git a/src/ncp/rcp_host.hpp b/src/ncp/rcp_host.hpp index 6f216310602..c896ee6601b 100644 --- a/src/ncp/rcp_host.hpp +++ b/src/ncp/rcp_host.hpp @@ -255,10 +255,14 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro static void DisableThreadAfterDetach(void *aContext); void DisableThreadAfterDetach(void); + static void SendMgmtPendingSetCallback(otError aError, void *aContext); + void SendMgmtPendingSetCallback(otError aError); bool IsAutoAttachEnabled(void); void DisableAutoAttach(void); + bool IsAttached(void); + otError SetOtbrAndOtLogLevel(otbrLogLevel aLevel); otInstance *mInstance; @@ -271,6 +275,7 @@ class RcpHost : public MainloopProcessor, public ThreadHost, public OtNetworkPro bool mEnableAutoAttach = false; AsyncResultReceiver mSetThreadEnabledReceiver; + AsyncResultReceiver mScheduleMigrationReceiver; #if OTBR_ENABLE_FEATURE_FLAGS // The applied FeatureFlagList in ApplyFeatureFlagList call, used for debugging purpose. diff --git a/tests/gtest/test_rcp_host_api.cpp b/tests/gtest/test_rcp_host_api.cpp index 9e78b77f5ad..4f49257fd9c 100644 --- a/tests/gtest/test_rcp_host_api.cpp +++ b/tests/gtest/test_rcp_host_api.cpp @@ -181,3 +181,55 @@ TEST(RcpHostApi, SetCountryCodeWorkCorrectly) host.Deinit(); } + +TEST(RcpHostApi, StateChangesCorrectlyAfterScheduleMigration) +{ + otError error = OT_ERROR_NONE; + bool resultReceived = false; + otbr::MainloopContext mainloop; + otbr::Ncp::ThreadHost::AsyncResultReceiver receiver = [&resultReceived, &error](otError aError, + const std::string &aErrorMsg) { + OT_UNUSED_VARIABLE(aErrorMsg); + resultReceived = true; + error = aError; + }; + otbr::Ncp::RcpHost host("wpan0", std::vector(), /* aBackboneInterfaceName */ "", /* aDryRun */ false, + /* aEnableAutoAttach */ false); + + otOperationalDataset dataset; + otOperationalDatasetTlvs datasetTlvs; + + // 1. Call ScheduleMigration when host hasn't been initialized. + otbr::MainloopManager::GetInstance().RemoveMainloopProcessor( + &host); // Temporarily remove RcpHost because it's not initialized yet. + host.ScheduleMigration(datasetTlvs, receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_INVALID_STATE); + otbr::MainloopManager::GetInstance().AddMainloopProcessor(&host); + + host.Init(); + + // 2. Call ScheduleMigration when the device is not attached. + error = OT_ERROR_NONE; + resultReceived = false; + host.ScheduleMigration(datasetTlvs, receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_FAILED); + + // 3. Schedule migration to another network. + OT_UNUSED_VARIABLE(otDatasetCreateNewNetwork(ot::FakePlatform::CurrentInstance(), &dataset)); + otDatasetConvertToTlvs(&dataset, &datasetTlvs); + OT_UNUSED_VARIABLE(otDatasetSetActiveTlvs(ot::FakePlatform::CurrentInstance(), &datasetTlvs)); + error = OT_ERROR_NONE; + resultReceived = false; + host.SetThreadEnabled(true, receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 1, + [&host]() { return host.GetDeviceRole() != OT_DEVICE_ROLE_DETACHED; }); + EXPECT_EQ(host.GetDeviceRole(), OT_DEVICE_ROLE_LEADER); + + host.ScheduleMigration(datasetTlvs, receiver); + MainloopProcessUntil(mainloop, /* aTimeoutSec */ 0, [&resultReceived]() { return resultReceived; }); + EXPECT_EQ(error, OT_ERROR_NONE); + + host.Deinit(); +}