openthread / ot-commissioner

OpenThread Commissioner, a Thread commissioner for joining new Thread devices and managing Thread networks.
https://openthread.io/
BSD 3-Clause "New" or "Revised" License
52 stars 35 forks source link

[udp-proxy] track Mesh-Local prefix in UDP Proxy client #188

Closed wgtdkp closed 3 years ago

wgtdkp commented 3 years ago

Previous implementation requires the caller of the Commissioner library to set Mesh-Local prefix before initiating MGMT commands. it is error-prone and breaks backward compatibility: we need to first fetch the Active Operational Dataset and set the Mesh-Local Prefix to the commissioner library before sending other MGMT_* commands: https://github.com/openthread/ot-commissioner/blob/1b5dd47875fdd52e86dd6334973c36fa9e1ba899/src/app/commissioner_app.cpp#L151-L155

This PR refactors the UDP Proxy to have it manage the Mesh-Local prefix inside the ProxyClient class.

  1. The Mesh-Local prefix is lazily requested from the Border Agent before sending the first MGMT command (if no Mesh-Local prefix is cached).
  2. The cached Mesh-Local prefix is cleared when the commissioner receives MGMT_DATASET_CHANGED.ntf so that the latest Mesh-Local prefix will be fetched before next MGMT command.
  3. The cached Mesh-Local prefix is cleared when the commissioner is disconnected.

In this way, the user doesn't need to be aware of the Mesh-Local prefix and no changes are required.

codecov-io commented 3 years ago

Codecov Report

Merging #188 (c8efa67) into main (f41edbd) will increase coverage by 0.00%. The diff coverage is 93.33%.

:exclamation: Current head c8efa67 differs from pull request most recent head de34d98. Consider uploading reports for the commit de34d98 to get more accurate results

@@           Coverage Diff           @@
##             main     #188   +/-   ##
=======================================
Coverage   70.35%   70.36%           
=======================================
Files          52       52           
Lines        4780     4778    -2     
=======================================
- Hits         3363     3362    -1     
+ Misses       1417     1416    -1     
Impacted Files Coverage Δ
include/commissioner/commissioner.hpp 43.75% <ø> (ø)
src/app/commissioner_app.cpp 39.65% <ø> (-0.10%) :arrow_down:
src/library/commissioner_impl.hpp 96.29% <ø> (ø)
src/library/commissioner_safe.cpp 79.92% <ø> (-0.31%) :arrow_down:
src/library/commissioner_safe.hpp 100.00% <ø> (ø)
src/library/commissioner_impl.cpp 75.91% <71.42%> (-0.66%) :arrow_down:
src/library/udp_proxy.cpp 91.13% <97.29%> (+5.42%) :arrow_up:
src/library/udp_proxy.hpp 91.66% <100.00%> (-1.20%) :arrow_down:
codecov-commenter commented 3 years ago

Codecov Report

Merging #188 (257d980) into main (1b5dd47) will increase coverage by 0.97%. The diff coverage is 93.87%.

:exclamation: Current head 257d980 differs from pull request most recent head d06e7c3. Consider uploading reports for the commit d06e7c3 to get more accurate results

@@            Coverage Diff             @@
##             main     #188      +/-   ##
==========================================
+ Coverage   69.40%   70.38%   +0.97%     
==========================================
Files          52       52              
Lines        4874     4782      -92     
==========================================
- Hits         3383     3366      -17     
+ Misses       1491     1416      -75     
Impacted Files Coverage Δ
include/commissioner/commissioner.hpp 43.75% <ø> (ø)
src/app/commissioner_app.cpp 39.65% <ø> (-0.10%) :arrow_down:
src/library/commissioner_impl.hpp 96.29% <ø> (ø)
src/library/commissioner_safe.cpp 79.92% <ø> (-0.31%) :arrow_down:
src/library/commissioner_safe.hpp 100.00% <ø> (ø)
src/library/commissioner_impl.cpp 75.91% <71.42%> (-1.09%) :arrow_down:
src/library/udp_proxy.cpp 91.56% <97.56%> (+5.85%) :arrow_up:
src/library/udp_proxy.hpp 91.66% <100.00%> (-1.20%) :arrow_down:
src/library/coap.hpp 95.29% <0.00%> (-1.18%) :arrow_down:
... and 4 more