Skip to content

Minor SignalClient improvements #662

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

Merged
merged 5 commits into from
Apr 8, 2025
Merged

Conversation

hiroshihorie
Copy link
Member

  • Send pingReq requests
  • Use state lock

@hiroshihorie hiroshihorie requested review from pblazej and Copilot April 7, 2025 16:44
Copy link

ilo-nanpa bot commented Apr 7, 2025

it seems like you haven't added any nanpa changeset files to this PR.

if this pull request includes changes to code, make sure to add a changeset, by writing a file to .nanpa/<unique-name>.kdl:

minor type="added" "Introduce frobnication algorithm"

refer to the manpage for more information.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

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

Comments suppressed due to low confidence (2)

Sources/LiveKit/Support/AsyncTimer.swift:90

  • The startIfStopped() method was removed; please ensure that replacing its usage with restart() elsewhere maintains the intended behavior, as the conditional start logic might differ.
func startIfStopped() { ... }

Sources/LiveKit/Core/SignalClient.swift:639

  • Replacing startIfStopped() with restart() for the ping timeout timer may change the timer's behavior; confirm that restart() correctly handles cases where the timer should only start if stopped.
_pingTimeoutTimer.restart()

@hiroshihorie hiroshihorie requested a review from ladvoc April 8, 2025 04:50
Copy link
Contributor

@ladvoc ladvoc left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@hiroshihorie hiroshihorie merged commit 1477cdf into main Apr 8, 2025
17 of 19 checks passed
@hiroshihorie hiroshihorie deleted the hiroshi/update-signalclient branch April 8, 2025 07:05
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.

2 participants