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

[application] move ThreadHost::Create out of the Application class #2574

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

Irving-cl
Copy link
Contributor

This PR refactors the Application class.

In specific:

  1. Decouple Application with InfraLinkSelector by moving InfraLinkSelector out of the Application class. The current dependency causes that we cannot move the creation of ThreadHost instance out of the constructor of the Application class.
  2. Move Ncp::ThreadHost::Create out of the Application constructor.

This change solves the fuzzer test issue in Android: When we create an Application object, we have to initlialize the radio which will cause error in the fuzz environment. With this change, we can pass a FuzzThreadHost object to the Application.

@Irving-cl Irving-cl changed the title [application] refactor to decouple Application with InfraLinkSelector [application] move ThreadHost::Create out of the Application class Oct 31, 2024
@Irving-cl Irving-cl force-pushed the refactor_application branch from 7927889 to 472a121 Compare October 31, 2024 12:07
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

Attention: Patch coverage is 84.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 46.15%. Comparing base (2b41187) to head (472a121).
Report is 859 commits behind head on main.

Files with missing lines Patch % Lines
src/agent/application.cpp 80.00% 0 Missing and 3 partials ⚠️
src/agent/main.cpp 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2574      +/-   ##
==========================================
- Coverage   55.77%   46.15%   -9.63%     
==========================================
  Files          87      105      +18     
  Lines        6890    12454    +5564     
  Branches        0      908     +908     
==========================================
+ Hits         3843     5748    +1905     
- Misses       3047     6394    +3347     
- Partials        0      312     +312     

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

Copy link
Contributor

@zhanglongxia zhanglongxia left a comment

Choose a reason for hiding this comment

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

LGTM

@Irving-cl Irving-cl requested a review from jwhui November 6, 2024 05:34
@Irving-cl Irving-cl added P2 and removed P1 labels Nov 6, 2024
@jwhui jwhui merged commit 10756a6 into openthread:main Nov 6, 2024
32 checks passed
@Irving-cl Irving-cl deleted the refactor_application branch November 6, 2024 11:58
andy31415 pushed a commit to andy31415/connectedhomeip that referenced this pull request Nov 6, 2024
This is after openthread/ot-br-posix#2574
removed the log line we were looking for.
andy31415 added a commit to project-chip/connectedhomeip that referenced this pull request Nov 6, 2024
This is after openthread/ot-br-posix#2574
removed the log line we were looking for.

Co-authored-by: Andrei Litvin <[email protected]>
yunhanw-google pushed a commit to yunhanw-google/connectedhomeip that referenced this pull request Nov 16, 2024
This is after openthread/ot-br-posix#2574
removed the log line we were looking for.

Co-authored-by: Andrei Litvin <[email protected]>
andy31415 added a commit to project-chip/connectedhomeip that referenced this pull request Nov 18, 2024
This is after openthread/ot-br-posix#2574
removed the log line we were looking for.

Co-authored-by: Andrei Litvin <[email protected]>
Co-authored-by: Andrei Litvin <[email protected]>
yyzhong-g pushed a commit to yyzhong-g/connectedhomeip that referenced this pull request Dec 12, 2024
This is after openthread/ot-br-posix#2574
removed the log line we were looking for.

Co-authored-by: Andrei Litvin <[email protected]>
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.

4 participants