Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ncp] add initial support of NCP #2283

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Irving-cl
Copy link
Contributor

@Irving-cl Irving-cl commented May 11, 2024

This PR adds initial support of NCP in otbr-agent. When otbr-agent
starts, it will detect the Co-processor type. If the Co-processor is
NCP, otbr-agent will still run. But at this moment, it will provide
a dbus API to get the device role of the NCP, which is 'Disabled'
because no other functions are supported now.

This PR implements NcpSpinel which is the main class to communicate
with the NCP. Different from RadioSpinel in openthread, NcpSpinel
is totally async. There are no blocking operations like WaitResponse
in RadioSpinel.

Currently when the Co-processor type is NCP, most modules in otbr-agent
will be initialized and run. These may be added gradually later. And
the dbus server also initiates differently when Co-processor type is NCP.
When it's NCP, the dbus server registers AsyncGetPropertyHandler
instead of GetPropertyHandler used now.

Depends-on openthread/openthread#10272

@Irving-cl
Copy link
Contributor Author

@bukepo @zhanglongxia @abtink @superwhd @wgtdkp

I think the change is a non-trivial one. The PR is still under development. There are many things to do: adding test, refactoring. But I hope that I can get some feedback on the framework and the direction in early stage. PTAL when you have a moment.

@Irving-cl Irving-cl force-pushed the add_ncp_spinel branch 18 times, most recently from fc93d3d to 311bbb6 Compare May 17, 2024 07:12
@Irving-cl Irving-cl force-pushed the add_ncp_spinel branch 5 times, most recently from 607b9e6 to 340be32 Compare May 20, 2024 05:14
@Irving-cl
Copy link
Contributor Author

This PR is ready for review now. It seems to be difficult to break
the PR down into smaller PRs. Here is an abstraction of what the PR
does:

  1. Defined an interface (IControllerOpenThread) where we place the
    unified thread control APIs (for both NCP and RCP cases). In this PR,
    only one API (GetDeviceRole) is added for testing.
  2. The original core class ControllerOpenThread is renamed as ControllerOpenThreadRcp
    and refactor'ed as an implementation class of IControllerOpenThread.
    Most of its original methods still exist.
  3. Another class ControllerOpenThreadNcp is added to implement IControllerOpenThread
    and is used under NCP case.
  4. The dependencies on ControllerOpenThread of the current modules
    are not changed at this staged. For example, BackboneAgent, NdProxyManager,
    BorderAgent all depend on an instance of ControllerOpenThread.
    Now they still depend on ControllerOpenThreadRcp, instead of IControllerOpenThread.
    Currently these modules won't be initialized and started under NCP
    mode. We may change this when later working on enabling these modules
    under NCP mode.
  5. Refactor'ed the initialization process. Currently the modules hold
    a reference to ControllerOpenThread. And the reference is passed
    in the class constructors. In this PR, th reference is changed to
    pointer and pointer is passed in Init method because the instance
    is created later based on the co-processor type.
  6. Renamed the variable Ncp. Currently the variable name for an
    instance of ControllerOpenThread is mNcp or aNcp. This is
    confusing from my perspective. The PR changes all these names to
    Ctrlr (Controller).
  7. Added a test case to test the dbus API under NCP mode.

There is still a pending PR in openthread. Without it the CI here
fails. Please take a loook when you have a chance.
@zhanglongxia @abtink @superwhd

@Irving-cl Irving-cl marked this pull request as ready for review May 20, 2024 05:59
@Irving-cl Irving-cl added the P2 label May 20, 2024
@Irving-cl
Copy link
Contributor Author

Per discussion, the PR is still too large for review. I'll try to breakdown it.

The first breakdown PR is: #2294

Please review the simple one first. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

1 participant