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

[rest] add joiner to commissioner joiner table and get status of joiner … #2517

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

martinzi
Copy link
Contributor

@martinzi martinzi commented Sep 30, 2024

…by indirect 'addThreadDeviceTask' processing

This PR is a carve-out from PR2514 to ease review. It implements the 'addThreadDeviceTask' to add a Joiner and auto-start on-mesh commissioner after POST:

  • POST addThreadDeviceTask on api/actions/
  • GET api/actions and GET api/actions/actionId
  • DELETE api/actions

The commit also provides limited integration tests, see tests/restjsonapi.

Please follow below steps to reproduce and install/build OTBR.

  1. Checkout this PR

  2. Build and Install OTBR as usual, e.g. on a Raspberry Pi

  3. Restart the OTBR. sudo systemctl restart otbr-agent

  4. To monitor the log [Errors|Warnings|Info] please open a different terminal instance and use following command:

tail -f /var/log/syslog | grep otbr
  1. Send POST request using Bruno or cURL to join a new device into your network.
curl -X POST -H 'Content-Type: application/vnd.api+json' http://localhost:8081/api/actions -d '{"data": [{"type": "addThreadDeviceTask", "attributes": {"eui": "6234567890AACDEA", "pskd": "J01NME", "timeout": 3600}}]}' | jq

should return

{
  "data": [
    {
      "id": "2d5a8844-b1bc-4f02-93f0-d87b8c3b4e92",
      "type": "addThreadDeviceTask",
      "attributes": {
        "eui": "6234567890AACDEB",
        "pskd": "J01NME",
        "timeout": 3600,
        "status": "pending"
      },
    }
  ]
}
  1. You may check the status and get the full collection of actions.
curl -X GET -H 'Accept: application/vnd.api+json' http://localhost:8081/api/actions | jq

should return

{
  "data": [
    {
      "id": "2d5a8844-b1bc-4f02-93f0-d87b8c3b4e92",
      "type": "addThreadDeviceTask",
      "attributes": {
        "eui": "6234567890AACDEB",
        "pskd": "J01NME",
        "timeout": 3600,
        "status": "pending"
      }
    }
  ],
  "meta": {
    "collection": {
      "offset": 0,
      "limit": 100,
      "total": 1
    }
  }
}
  1. View the entry added to the commissioner's table sudo ot-ctl commissioner joiner table and expect
| ID                    | PSKd                             | Expiration |
+-----------------------+----------------------------------+------------+
|      6234567890aacdea |                           J01NME |    3459027 |
Done
  1. Start your joiner and after a few seconds repeat above steps 6. and 7.

  2. For running the included test script install Bruno-Cli and run the bash script on your border router

cd tests/restjsonapi
source ./install_bruno_cli
./test-restjsonapi-server

…by indirect 'addThreadDeviceTask' processing

This commit implements
- POST `addThreadDeviceTask` on api/actions/
- GET api/actions/<actionId>
- DELETE api/actions

The commit also provides integration tests, see tests/restjsonapi.

Please follow the following steps to install/build OTBR.

1. Checkout this PR

2. Build and Install OTBR as usual, e.g. on a Raspberry Pi

3. Restart the OTBR. `sudo systemctl restart otbr-agent`

4. To monitor the log [Errors|Warnings|Info] please open a different terminal instance and use following command:

```
tail -f /var/log/syslog | grep otbr
```

5. Send POST request using BRUNO or CURL, e.g. to join a new device into your network.

```
curl -X POST -H 'Content-Type: application/vnd.api+json' http://localhost:8081/api/actions -d '{"data": [{"type": "addThreadDeviceTask", "attributes": {"eui": "6234567890AACDEA", "pskd": "J01NME", "timeout": 3600}}]}' | jq
```

should return

```
{
  "data": [
    {
      "id": "2d5a8844-b1bc-4f02-93f0-d87b8c3b4e92",
      "type": "addThreadDeviceTask",
      "attributes": {
        "eui": "6234567890AACDEB",
        "pskd": "J01NME",
        "timeout": 3600,
        "status": "pending"
      },
    }
  ]
}
```

6. You may check the status and get the full collection of actions.

```
curl -X GET -H 'Accept: application/vnd.api+json' http://localhost:8081/api/actions | jq
```

should return

```
{
  "data": [
    {
      "id": "2d5a8844-b1bc-4f02-93f0-d87b8c3b4e92",
      "type": "addThreadDeviceTask",
      "attributes": {
        "eui": "6234567890AACDEB",
        "pskd": "J01NME",
        "timeout": 3600,
        "status": "pending"
      }
    }
  ],
  "meta": {
    "collection": {
      "offset": 0,
      "limit": 100,
      "total": 1
    }
  }
}
```

7. View the entry added to the commissioner's table `sudo ot-ctl commissioner joiner table` and expect

```
| ID                    | PSKd                             | Expiration |
+-----------------------+----------------------------------+------------+
|      6234567890aacdea |                           J01NME |    3459027 |
Done
```

8. Start your joiner and after a few seconds repeat above steps 6. and 7.

9. For running the included test script install Bruno-Cli and run the bash script on your border router

```
cd tests/restjsonapi
source ./install_bruno_cli
./test-restjsonapi-server
```
Copy link

codecov bot commented Sep 30, 2024

Codecov Report

Attention: Patch coverage is 7.61180% with 971 lines in your changes missing coverage. Please review.

Project coverage is 42.93%. Comparing base (2b41187) to head (3c37296).
Report is 856 commits behind head on main.

Files with missing lines Patch % Lines
src/rest/extensions/commissioner_allow_list.cpp 0.00% 260 Missing ⚠️
src/rest/extensions/rest_task_queue.cpp 2.64% 183 Missing and 1 partial ⚠️
src/rest/resource.cpp 15.47% 141 Missing and 1 partial ⚠️
...rc/rest/extensions/rest_task_add_thread_device.cpp 0.00% 111 Missing ⚠️
src/rest/extensions/rest_server_common.cpp 0.00% 57 Missing ⚠️
src/rest/extensions/rest_task_handler.cpp 0.00% 55 Missing ⚠️
src/rest/extensions/uuid.cpp 0.00% 47 Missing ⚠️
src/rest/extensions/linked_list.hpp 4.34% 44 Missing ⚠️
src/rest/parser.cpp 63.79% 19 Missing and 2 partials ⚠️
src/rest/extensions/commissioner_allow_list.hpp 0.00% 20 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2517       +/-   ##
===========================================
- Coverage   55.77%   42.93%   -12.85%     
===========================================
  Files          87      113       +26     
  Lines        6890    13482     +6592     
  Branches        0     1055     +1055     
===========================================
+ Hits         3843     5788     +1945     
- Misses       3047     7378     +4331     
- Partials        0      316      +316     

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

@wgtdkp
Copy link
Member

wgtdkp commented Oct 1, 2024

@martinzi Thanks for splitting this out!

A few suggestions:

  1. could you please update https://github.com/openthread/ot-br-posix/blame/main/src/rest/openapi.yaml for the new REST APIs? We use the OpenAPI Specification for defining our REST API, see https://swagger.io/specification/ for details.
  2. could you fix the CI test failures to make sure everything works before we getting into code details? thanks
  3. Your code pretty test is failing, this is likely because you didn't format your code. You can use the command script/make-pretty to format your code.

BRs!

@wgtdkp
Copy link
Member

wgtdkp commented Oct 9, 2024

Please be aware that there is also another PR adding the commissioner feature to REST API #2515

@martinzi martinzi force-pushed the rest-jsonapi-addJoiner branch 12 times, most recently from 0ff638e to 08f6c06 Compare November 4, 2024 16:55
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