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

[tcat] implement tcat advertisement #9858

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

canisLupus1313
Copy link
Contributor

Commit introduces implementation of TCAT advertisement over BLE compialnt with Thread 1.3.1 specification.

@canisLupus1313
Copy link
Contributor Author

@EskoDijk @arnulfrupp FYI

Copy link

size-report bot commented Feb 16, 2024

Size Report of OpenThread

Merging #9858 into main(d6eb56c).

name branch text data bss total
ot-cli-ftd main 466528 856 66332 533716
#9858 466528 856 66332 533716
+/- 0 0 0 0
ot-ncp-ftd main 435396 760 61544 497700
#9858 435396 760 61544 497700
+/- 0 0 0 0
libopenthread-ftd.a main 235681 95 40278 276054
#9858 235681 95 40278 276054
+/- 0 0 0 0
libopenthread-cli-ftd.a main 57589 0 8075 65664
#9858 57589 0 8075 65664
+/- 0 0 0 0
libopenthread-ncp-ftd.a main 31863 0 5916 37779
#9858 31863 0 5916 37779
+/- 0 0 0 0
ot-cli-mtd main 364528 760 51204 416492
#9858 364528 760 51204 416492
+/- 0 0 0 0
ot-ncp-mtd main 347012 760 46432 394204
#9858 347012 760 46432 394204
+/- 0 0 0 0
libopenthread-mtd.a main 157892 0 25166 183058
#9858 157892 0 25166 183058
+/- 0 0 0 0
libopenthread-cli-mtd.a main 39812 0 8059 47871
#9858 39812 0 8059 47871
+/- 0 0 0 0
libopenthread-ncp-mtd.a main 24743 0 5916 30659
#9858 24743 0 5916 30659
+/- 0 0 0 0
ot-cli-ftd-br main 549872 864 131172 681908
#9858 549872 864 131172 681908
+/- 0 0 0 0
libopenthread-ftd-br.a main 322982 100 105094 428176
#9858 322982 100 105094 428176
+/- 0 0 0 0
libopenthread-cli-ftd-br.a main 71450 0 8099 79549
#9858 71450 0 8099 79549
+/- 0 0 0 0
ot-rcp main 62280 564 20604 83448
#9858 62280 564 20604 83448
+/- 0 0 0 0
libopenthread-rcp.a main 9542 0 5052 14594
#9858 9542 0 5052 14594
+/- 0 0 0 0
libopenthread-radio.a main 19042 0 214 19256
#9858 19042 0 214 19256
+/- 0 0 0 0

@canisLupus1313 canisLupus1313 force-pushed the bbtc_advertisement branch 2 times, most recently from f1f37a8 to 8f6d29a Compare February 16, 2024 16:59
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

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

Project coverage is 82.77%. Comparing base (74573b5) to head (fe8103f).
Report is 2 commits behind head on main.

❗ Current head fe8103f differs from pull request most recent head fc8db35. Consider uploading reports for the commit fc8db35 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #9858       +/-   ##
===========================================
+ Coverage   64.92%   82.77%   +17.84%     
===========================================
  Files         505      560       +55     
  Lines       61703    72098    +10395     
===========================================
+ Hits        40062    59678    +19616     
+ Misses      21641    12420     -9221     
Files Coverage Δ
tests/unit/test_platform.cpp 40.00% <0.00%> (-11.86%) ⬇️

... and 533 files with indirect coverage changes

@canisLupus1313 canisLupus1313 force-pushed the bbtc_advertisement branch 9 times, most recently from 69af197 to fe8103f Compare February 18, 2024 13:15
include/openthread/tcat.h Outdated Show resolved Hide resolved
include/openthread/tcat.h Outdated Show resolved Hide resolved
include/openthread/tcat.h Outdated Show resolved Hide resolved
include/openthread/tcat.h Outdated Show resolved Hide resolved
include/openthread/tcat.h Outdated Show resolved Hide resolved
include/openthread/tcat.h Outdated Show resolved Hide resolved
src/cli/cli_tcat.cpp Outdated Show resolved Hide resolved
Copy link
Member

@abtink abtink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @canisLupus1313. Looks good overall.
Some smaller suggestions below.

src/core/meshcop/tcat_agent.cpp Outdated Show resolved Hide resolved
src/core/meshcop/tcat_agent.cpp Outdated Show resolved Hide resolved
src/core/meshcop/tcat_agent.cpp Outdated Show resolved Hide resolved
src/core/meshcop/tcat_agent.cpp Outdated Show resolved Hide resolved
include/openthread/platform/ble.h Outdated Show resolved Hide resolved
src/cli/cli_tcat.cpp Outdated Show resolved Hide resolved
src/cli/cli_tcat.cpp Outdated Show resolved Hide resolved
src/cli/cli_tcat.cpp Outdated Show resolved Hide resolved
src/core/meshcop/tcat_agent.cpp Outdated Show resolved Hide resolved
src/core/meshcop/tcat_agent.hpp Outdated Show resolved Hide resolved
@canisLupus1313 canisLupus1313 force-pushed the bbtc_advertisement branch 4 times, most recently from 7b25679 to 292d6d5 Compare May 21, 2024 08:22
@canisLupus1313
Copy link
Contributor Author

@EskoDijk @arnulfrupp any more comments from Your site?

@canisLupus1313 canisLupus1313 force-pushed the bbtc_advertisement branch 5 times, most recently from 71cd7b7 to fa351ef Compare May 22, 2024 14:06
Copy link
Member

@abtink abtink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks @canisLupus1313.

Some smaller suggestion below.

include/openthread/platform/ble.h Show resolved Hide resolved
include/openthread/platform/ble.h Outdated Show resolved Hide resolved
src/core/meshcop/tcat_agent.cpp Outdated Show resolved Hide resolved
src/core/radio/ble_secure.cpp Show resolved Hide resolved
@EskoDijk
Copy link
Contributor

@EskoDijk @arnulfrupp any more comments from Your site?

@canisLupus1313 Not for now - maybe I can propose further changes (if needed) in a later PR or issue. It's easier to see what's needed after some test runs (either in simulation or on the testbed).

@canisLupus1313 canisLupus1313 force-pushed the bbtc_advertisement branch 2 times, most recently from a47ca03 to 6c6d653 Compare May 22, 2024 21:38
@canisLupus1313
Copy link
Contributor Author

@EskoDijk @arnulfrupp any more comments from Your site?

@canisLupus1313 Not for now - maybe I can propose further changes (if needed) in a later PR or issue. It's easier to see what's needed after some test runs (either in simulation or on the testbed).

@EskoDijk
Can I count on +1 from you then?

@canisLupus1313 canisLupus1313 force-pushed the bbtc_advertisement branch 2 times, most recently from 4be06c5 to db8a1bf Compare May 22, 2024 21:56
Copy link
Contributor

@EskoDijk EskoDijk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks ok (I didn't review all details)

Commit introduces implementation of TCAT advertisement over BLE compialnt with
Thread 1.3.1 specification.
@canisLupus1313
Copy link
Contributor Author

@jwhui @abtink Looks like this PR has already positive review. Can we merge it?

@jwhui jwhui changed the title [tcat] Implement tcat advertisement. [tcat] Implement tcat advertisement Jun 5, 2024
@jwhui jwhui changed the title [tcat] Implement tcat advertisement [tcat] implement tcat advertisement Jun 5, 2024
@jwhui jwhui self-requested a review June 5, 2024 21:52
@jwhui jwhui merged commit 398df8c into openthread:main Jun 6, 2024
103 checks passed
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.

None yet

5 participants