Skip to content

Conversation

gmarcosb
Copy link
Contributor

@gmarcosb gmarcosb commented Oct 17, 2025

Summary

Address outstanding TCP cleanup

Related issues

Fixes #41453
Fixes #41404

Testing

CI

Readability checklist

The checklist below will help the reviewer finish PR review in time and keep the
code readable:

  • PR title is
    descriptive
  • Apply the
    “When in Rome…”
    rule (coding style)
  • PR size is short
  • Try to avoid "squashing" and "force-update" in commit history
  • CI time didn't increase

See: Pull Request Guidelines

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request aims to improve the lifetime management of TCPEndPoint objects by consistently using TCPEndPointHandle in callbacks. The changes are generally good and follow this pattern. However, I've found a critical issue in the LwIP implementation (TCPEndPointImplLwIP.cpp) where some function calls were not updated to their new signatures, which will likely lead to compilation errors. I've also pointed out a minor typo in a comment. Please see the detailed comments for suggestions.

@andy31415
Copy link
Contributor

@gmarcosb could you describe more what Address outstanding TCP cleanup means?

What where the changes, why, why are they the correct ones?

@gmarcosb gmarcosb requested a review from pidarped October 17, 2025 18:31
@gmarcosb
Copy link
Contributor Author

@gmarcosb could you describe more what Address outstanding TCP cleanup means?

What where the changes, why, why are they the correct ones?

It's covered in the 2 issues linked to the PR which this PR will fix; the reason for this PR is to address those 2 outstanding issues (thus outstanding TCP cleanup)

Copy link

github-actions bot commented Oct 17, 2025

PR #41520: Size comparison from c87ece5 to 04ee89b

Full report (34 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
platform target config section c87ece5 04ee89b change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1106314 1106314 0 0.0
RAM 178802 178802 0 0.0
bl702 lighting-app bl702+eth FLASH 660904 660904 0 0.0
RAM 134881 134881 0 0.0
bl702+wifi FLASH 837016 837016 0 0.0
RAM 124349 124349 0 0.0
bl706+mfd+rpc+littlefs FLASH 1069984 1069984 0 0.0
RAM 117189 117189 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 898826 898826 0 0.0
RAM 105468 105468 0 0.0
lighting-app bl702l+mfd+littlefs FLASH 983002 983002 0 0.0
RAM 109676 109676 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 770324 770324 0 0.0
RAM 103240 103240 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 782080 782080 0 0.0
RAM 108400 108400 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 727900 727900 0 0.0
RAM 97308 97308 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 712344 712344 0 0.0
RAM 97508 97508 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 554026 553990 -36 -0.0
RAM 205504 205504 0 0.0
lock CC3235SF_LAUNCHXL FLASH 587278 587242 -36 -0.0
RAM 205768 205768 0 0.0
efr32 lock-app BRD4187C FLASH 962880 962880 0 0.0
RAM 126268 126268 0 0.0
BRD4338a FLASH 756080 756080 0 0.0
RAM 256888 256888 0 0.0
window-app BRD4187C FLASH 1057812 1057804 -8 -0.0
RAM 122464 122464 0 0.0
esp32 all-clusters-app c3devkit DRAM 103192 103192 0 0.0
FLASH 1795828 1795828 0 0.0
IRAM 83862 83862 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 932776 932776 0 0.0
RAM 161069 161069 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1675932 1675916 -16 -0.0
RAM 213660 213660 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1592540 1592524 -16 -0.0
RAM 210956 210956 0 0.0
light cy8ckit_062s2_43012 FLASH 1459156 1459140 -16 -0.0
RAM 197656 197656 0 0.0
lock cy8ckit_062s2_43012 FLASH 1491708 1491692 -16 -0.0
RAM 225376 225376 0 0.0
qpg lighting-app qpg6200+debug FLASH 836520 836520 0 0.0
RAM 127644 127644 0 0.0
lock-app qpg6200+debug FLASH 773220 773220 0 0.0
RAM 118620 118620 0 0.0
realtek light-switch-app rtl8777g FLASH 706208 706208 0 0.0
RAM 106800 106800 0 0.0
lighting-app rtl8777g FLASH 757304 757304 0 0.0
RAM 127164 127164 0 0.0
stm32 light STM32WB5MM-DK FLASH 469772 469772 0 0.0
RAM 141248 141248 0 0.0
telink bridge-app tl7218x FLASH 710402 710402 0 0.0
RAM 90436 90436 0 0.0
light-app-ota-compress-lzma-shell-factory-data tl3218x FLASH 796788 796788 0 0.0
RAM 40936 40936 0 0.0
light-app-ota-shell-factory-data tl7218x FLASH 787988 787988 0 0.0
RAM 93580 93580 0 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 714914 714914 0 0.0
RAM 51736 51736 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 748218 748218 0 0.0
RAM 70784 70784 0 0.0
light-switch-app-ota-factory-data tl3218x_retention FLASH 725066 725066 0 0.0
RAM 34484 34484 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 602304 602304 0 0.0
RAM 108628 108628 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 820608 820612 4 0.0
RAM 91976 91976 0 0.0

Copy link

github-actions bot commented Oct 17, 2025

PR #41520: Size comparison from c87ece5 to c2826f9

Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
platform target config section c87ece5 c2826f9 change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1106314 1106314 0 0.0
RAM 178802 178802 0 0.0
bl702 lighting-app bl702+eth FLASH 660904 660904 0 0.0
RAM 134881 134881 0 0.0
bl702+wifi FLASH 837016 837016 0 0.0
RAM 124349 124349 0 0.0
bl706+mfd+rpc+littlefs FLASH 1069984 1069984 0 0.0
RAM 117189 117189 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 898826 898826 0 0.0
RAM 105468 105468 0 0.0
lighting-app bl702l+mfd+littlefs FLASH 983002 983002 0 0.0
RAM 109676 109676 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 770324 770324 0 0.0
RAM 103240 103240 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 782080 782080 0 0.0
RAM 108400 108400 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 727900 727900 0 0.0
RAM 97308 97308 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 712344 712344 0 0.0
RAM 97508 97508 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 554026 553990 -36 -0.0
RAM 205504 205504 0 0.0
lock CC3235SF_LAUNCHXL FLASH 587278 587242 -36 -0.0
RAM 205768 205768 0 0.0
efr32 lock-app BRD4187C FLASH 962880 962880 0 0.0
RAM 126268 126268 0 0.0
BRD4338a FLASH 756080 756080 0 0.0
RAM 256888 256888 0 0.0
window-app BRD4187C FLASH 1057812 1057804 -8 -0.0
RAM 122464 122464 0 0.0
esp32 all-clusters-app c3devkit DRAM 103192 103192 0 0.0
FLASH 1795828 1795828 0 0.0
IRAM 83862 83862 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 932776 932776 0 0.0
RAM 161069 161069 0 0.0
nxp contact mcxw71+release FLASH 691360 691360 0 0.0
RAM 61424 61424 0 0.0
lighting mcxw71+release FLASH 722856 722856 0 0.0
RAM 68084 68084 0 0.0
lock mcxw71+release FLASH 773128 773128 0 0.0
RAM 61868 61868 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1675932 1675916 -16 -0.0
RAM 213660 213660 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1592540 1592524 -16 -0.0
RAM 210956 210956 0 0.0
light cy8ckit_062s2_43012 FLASH 1459156 1459140 -16 -0.0
RAM 197656 197656 0 0.0
lock cy8ckit_062s2_43012 FLASH 1491708 1491692 -16 -0.0
RAM 225376 225376 0 0.0
qpg lighting-app qpg6200+debug FLASH 836520 836520 0 0.0
RAM 127644 127644 0 0.0
lock-app qpg6200+debug FLASH 773220 773220 0 0.0
RAM 118620 118620 0 0.0
realtek light-switch-app rtl8777g FLASH 706208 706208 0 0.0
RAM 106800 106800 0 0.0
lighting-app rtl8777g FLASH 757304 757304 0 0.0
RAM 127164 127164 0 0.0
stm32 light STM32WB5MM-DK FLASH 469772 469772 0 0.0
RAM 141248 141248 0 0.0
telink bridge-app tl7218x FLASH 710402 710402 0 0.0
RAM 90436 90436 0 0.0
light-app-ota-compress-lzma-shell-factory-data tl3218x FLASH 796788 796788 0 0.0
RAM 40936 40936 0 0.0
light-app-ota-shell-factory-data tl7218x FLASH 787988 787988 0 0.0
RAM 93580 93580 0 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 714914 714914 0 0.0
RAM 51736 51736 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 748218 748218 0 0.0
RAM 70784 70784 0 0.0
light-switch-app-ota-factory-data tl3218x_retention FLASH 725066 725066 0 0.0
RAM 34484 34484 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 602304 602304 0 0.0
RAM 108628 108628 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 820608 820612 4 0.0
RAM 91976 91976 0 0.0

Copy link

codecov bot commented Oct 17, 2025

Codecov Report

❌ Patch coverage is 87.50000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.01%. Comparing base (69f67fb) to head (c2826f9).
⚠️ Report is 41 commits behind head on master.

Files with missing lines Patch % Lines
src/inet/TCPEndPoint.h 0.00% 2 Missing ⚠️
src/inet/TCPEndPointImplSockets.cpp 66.66% 1 Missing ⚠️
src/transport/raw/TCP.cpp 95.83% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #41520      +/-   ##
==========================================
+ Coverage   51.00%   51.01%   +0.01%     
==========================================
  Files        1385     1386       +1     
  Lines      100969   100980      +11     
  Branches    13079    13075       -4     
==========================================
+ Hits        51500    51519      +19     
+ Misses      49469    49461       -8     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify mergify bot merged commit 3e5d0a5 into project-chip:master Oct 21, 2025
79 of 80 checks passed
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.

TCPEndPoint::OnAcceptErrorFunct behavior unclear Use an EndPointHandle in OnConnectionClosedFunct

4 participants