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

[api] remove the XpanId and PanId structs #264

Merged
merged 2 commits into from
May 8, 2024

Conversation

wgtdkp
Copy link
Member

@wgtdkp wgtdkp commented Apr 29, 2024

It's an overkill to define a dedicated class/struct for a simple uint16_t
just for pretty print to json in HEX string.

The library interface should prefer native/primitive data types for better
interoperability with other languages such as java and Objective-C

This commit basically reverts the XpanId and PanId changes introduced in #202

@wgtdkp wgtdkp force-pushed the remove-panid-struct branch 11 times, most recently from 71fd9a4 to ef31c91 Compare May 1, 2024 15:55
It's an overkill to define a dedicated class/struct for a simple
`uint16_t` just for pretty print to json in HEX string.
@wgtdkp wgtdkp force-pushed the remove-panid-struct branch from ef31c91 to dbfa960 Compare May 6, 2024 14:58
@wgtdkp wgtdkp changed the title [API] remove the PanId struct [API] remove the XpanId and PanId struct May 6, 2024
@wgtdkp wgtdkp added enhancement New feature or request library labels May 6, 2024
@wgtdkp wgtdkp requested review from sunytt and librasungirl May 6, 2024 15:00
@codecov-commenter
Copy link

codecov-commenter commented May 7, 2024

Codecov Report

Attention: Patch coverage is 76.08696% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 72.83%. Comparing base (79d1ddc) to head (143953c).
Report is 1 commits behind head on main.

❗ Current head 143953c differs from pull request most recent head 5cb6cc8. Consider uploading reports for the commit 5cb6cc8 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #264      +/-   ##
==========================================
- Coverage   72.88%   72.83%   -0.05%     
==========================================
  Files          72       72              
  Lines        7500     7476      -24     
==========================================
- Hits         5466     5445      -21     
+ Misses       2034     2031       -3     
Files Coverage Δ
src/app/commissioner_app.hpp 100.00% <ø> (ø)
src/app/json.cpp 81.26% <100.00%> (-0.18%) ⬇️
src/app/ps/registry.cpp 89.67% <100.00%> (+0.05%) ⬆️
src/app/ps/registry_entries.cpp 98.30% <100.00%> (+1.12%) ⬆️
src/app/ps/registry_entries.hpp 100.00% <ø> (ø)
src/common/error.cpp 96.00% <100.00%> (ø)
src/common/utils.hpp 100.00% <100.00%> (ø)
src/library/network_data.cpp 74.64% <ø> (-4.74%) ⬇️
src/app/cli/interpreter.cpp 79.40% <75.00%> (-0.07%) ⬇️
src/app/ps/persistent_storage_json.cpp 58.94% <50.00%> (ø)
... and 5 more

@wgtdkp wgtdkp force-pushed the remove-panid-struct branch from e840da8 to 5cb6cc8 Compare May 7, 2024 09:25
@wgtdkp wgtdkp requested review from abtink and removed request for sunytt May 7, 2024 09:51
@wgtdkp
Copy link
Member Author

wgtdkp commented May 7, 2024

@abtink I am not sure if you are willing to review the ot-commissioner changes :)

In case no, please feel free to remove yourself from the pending list, I can wait for rongli :)

@wgtdkp wgtdkp requested review from jwhui and removed request for abtink May 8, 2024 05:57
@jwhui jwhui changed the title [API] remove the XpanId and PanId struct [api] remove the XpanId and PanId structs May 8, 2024
@jwhui jwhui merged commit 7b12b9f into openthread:main May 8, 2024
33 checks passed
ZhangLe2016 pushed a commit to ZhangLe2016/ot-commissioner that referenced this pull request Jul 16, 2024
It's an overkill to define a dedicated class/struct for a simple
`uint16_t` just for pretty print to json in HEX string.
ZhangLe2016 pushed a commit to ZhangLe2016/ot-commissioner that referenced this pull request Jul 16, 2024
It's an overkill to define a dedicated class/struct for a simple
`uint16_t` just for pretty print to json in HEX string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants