镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Re-pin sendspin-cpp to 3a3722a8 and read pairing from is_paired() - #115

Open
chrisuthe wants to merge 1 commit into
mainfrom
chore/repin-sendspin-cpp-3a3722a8
Open

chrisuthe wants to merge 1 commit into
mainfrom
chore/repin-sendspin-cpp-3a3722a8

Conversation

@chrisuthe

Copy link
Copy Markdown
Member

Why

sendspin-cpp#218 merged as 3a3722a8. It adds SendspinClientConfig::application_roles, which #110 needs for local pairing. Moving the pin there also brings sendspin-cpp#234, which removes ConnectionTrust and on_trust_changed() in favour of SendspinClient::is_paired(), so the pin cannot move without this migration.

What

  • Pin 3a3722a8, 31 commits past b0fcf5bb.
  • PairingListener drops the callback and polls is_paired() once per main-loop tick, reading it before is_connected() as the library asks. It still logs Connected server is paired / not paired when a session is new or has been re-keyed, and still clears a stale pairing attempt then.
  • status reads is_paired() before is_connected() too. The trust line is paired, unpaired or not connected as before. unknown is gone: a connected session is always one or the other.
  • docs/ROADMAP.md item 3 names the new pin.

Compatibility

No flag, config key, control-socket message, state format or exit code changes. The only visible difference is that status no longer prints trust: unknown.

Testing

Ubuntu 24.04, Release, -DSENDSPIN_CLI_WERROR=ON, ALSA, PulseAudio, PipeWire, PortAudio and Avahi, fresh FetchContent:

  • scripts/format.sh --check: every file is formatted.
  • --version reports sendspin-cpp 3a3722a8….
  • ctest: 554 of 557 pass. The three failures are StateStore.AnUnwritableDirectoryFails, AFailedWriteIsNotRememberedAsASuccess and WritesVolumeAndMuteInOneGo. Each makes a directory read-only and expects a write to fail, and the run was as root, which writes regardless. This PR doesn't touch the state store, and CI runs them unprivileged.
  • scripts/smoke_test.sh: every check passed.

3a3722a8 is sendspin-cpp#218 as merged, which adds application-specific roles. It brings #234, which removes ConnectionTrust and on_trust_changed() for SendspinClient::is_paired(). The main loop now polls it and logs "Connected server is paired/not paired" when a session is new or re-keyed, as the callback did. status reads it before is_connected(), as the library asks. Its trust line loses "unknown": a connected session is always paired or not.

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.

🟡 Changes recommended

Clearing server_id_ on the first connected tick can discard a newly started pairing attempt and incorrectly report pairing: none; the callback ordering needs a fix and regression test.

1 open issue
What changed in this PR

Updates sendspin-cpp to 3a3722a8, enabling the application-role support needed for local pairing in #110, and migrates pairing state to the new is_paired() API.

Changes:

  • Re-pins the dependency and updates the roadmap reference.
  • Replaces trust callbacks with main-loop polling and direct pairing-state reads for status.
  • Updates status formatting and tests to remove the unknown trust state.
File Description
tests/​control_test.cpp Tests paired, unpaired, and disconnected status values.
src/​main.cpp Polls pairing state and migrates status snapshots to is_paired().
src/​control.h Replaces optional trust with a pairing boolean.
src/​control_common.cpp Formats trust using connection and pairing state.
docs/​ROADMAP.md Updates the documented dependency pin.
CMakeLists.txt Pins sendspin-cpp to 3a3722a8.

Review effort: Balanced

Comment thread src/main.cpp
Comment on lines +159 to +161
if (connected && (!this->connected_ || paired != this->paired_)) {
// A new handshake: an attempt the last connection dropped never reported its end.
this->server_id_.clear();
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