Skip to content

Set socket timeout to 5s#3929

Merged
jamesarich merged 2 commits into
meshtastic:mainfrom
weebl2000:increase-read-timeout
Dec 8, 2025
Merged

Set socket timeout to 5s#3929
jamesarich merged 2 commits into
meshtastic:mainfrom
weebl2000:increase-read-timeout

Conversation

@weebl2000

@weebl2000 weebl2000 commented Dec 7, 2025

Copy link
Copy Markdown
Contributor

500ms socket timeout is incredibly low, 5 seconds makes more sense. Kept the total timeout the same.

@jamesarich jamesarich requested a review from Copilot December 7, 2025 23:40
Copilot AI reviewed Dec 8, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@weebl2000 weebl2000 force-pushed the increase-read-timeout branch 2 times, most recently from b9aac32 to 81fbd9d Compare December 8, 2025 17:28
@weebl2000
Set socket timeout to 5s
44f5b18 
500ms socket timeout is incredibly low
@weebl2000 weebl2000 force-pushed the increase-read-timeout branch from 81fbd9d to 44f5b18 Compare December 8, 2025 17:28
@jamesarich jamesarich requested a review from Copilot December 8, 2025 18:09
Copilot AI reviewed Dec 8, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@jamesarich
refactor: extract constants for socket timeout and retries
aa370b0 
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
@jamesarich jamesarich enabled auto-merge December 8, 2025 20:08
@codecov

codecov Bot commented Dec 8, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.52%. Comparing base (9246b3b) to head (aa370b0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...m/geeksville/mesh/repository/radio/TCPInterface.kt 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@          Coverage Diff          @@
##            main   #3929   +/-   ##
=====================================
  Coverage   0.52%   0.52%           
=====================================
  Files        394     394           
  Lines      23136   23136           
  Branches    2933    2933           
=====================================
  Hits         122     122           
  Misses     22993   22993           
  Partials      21      21

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

@jamesarich

jamesarich commented Dec 8, 2025

Copy link
Copy Markdown
Collaborator

@weebl2000 i went ahead and fixed those detekt complaints, it looked like they had been previously suppressed/baselined (were not introduced by your changes, just were being hidden).

For future contributions please ensure that the code passes ./gradlew spotless detekt checks. Will save us from having to ask (and some ci churn). Thanks!

@jamesarich jamesarich added this pull request to the merge queue Dec 8, 2025
@weebl2000

weebl2000 commented Dec 8, 2025

Copy link
Copy Markdown
Contributor Author

@weebl2000 i went ahead and fixed those detekt complaints, it looked like they had been previously suppressed/baselined (were not introduced by your changes, just were being hidden).

For future contributions please ensure that the code passes ./gradlew spotless detekt checks. Will save us from having to ask (and some ci churn). Thanks!

Will keep it in mind 👌🏽

Merged via the queue into meshtastic:main with commit 2ad703c Dec 8, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@jamesarich jamesarich jamesarich approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@weebl2000 @jamesarich