Fix sendspin-jvm adapter for ServerState.metadata type change - #107
Open
OnFreund wants to merge 2 commits into
Open
Fix sendspin-jvm adapter for ServerState.metadata type change#107OnFreund wants to merge 2 commits into
OnFreund wants to merge 2 commits into
Conversation
sendspin-jvm's Sendspin/sendspin-jvm#33 wrapped ServerState.metadata/ controller/color in JsonOptional<T> to fix a bug where the client couldn't distinguish "role omitted" (leave unchanged) from "role explicitly null" (clear), per spec PR #175. This adapter read state.metadata as a plain TrackMetadataMsg?, breaking the build. Fixed using the same JsonOptional.orNull() idiom this file already uses elsewhere (e.g. meta.title.orNull() in addMetadataFields). Functionally equivalent to the old code for the non-null-clearing case this adapter cares about (tracking the last received metadata value). Verified: `python -m conformance.build` succeeds for sendspin-jvm-client and the aiosendspin_server smoke-test import passes locally. Could not exercise the full conformance matrix in this sandbox (client/server socket timeouts even against the previous, already-passing commit — a local networking limitation, not a regression from this change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dth/height rename
aiosendspin's own ArtworkChannel model (fixed in a companion PR to
Sendspin/aiosendspin) is being updated for spec PR #168 ("Letterbox art
and remove BMP support"): media_width/media_height -> width/height,
and BMP dropped from PictureFormat entirely. This adapter constructed
ArtworkChannel directly with the old kwarg names and referenced
PictureFormat.BMP, which would break once that aiosendspin fix lands.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related fixes, both surfaced by Sendspin/sendspin-jvm#33:
Build break:
Sendspin/sendspin-jvm#33wrappedServerState.metadata/controller/colorinJsonOptional<T>(fixing a real bug — the client couldn't distinguish "role omitted" from "role explicitlynull" per spec PR #175). That's source-breaking for this repo'ssendspin-jvmadapter (adapters/sendspin-jvm/client/src/main/kotlin/com/sendspin/conformance/Main.kt), which readstate.metadataas a plainTrackMetadataMsg?. Fixed with theJsonOptional.orNull()idiom this file already uses elsewhere.Matrix regression: with the build fixed, the full
aiosendspin -> sendspin-jvmmatrix started failing across the board (9/9 scenarios) — not a regression from fix Add sendspin-cpp conformance client adapter #1, but a separate, real interop gap: sendspin-jvm also picked up spec PR #168 (artworkmedia_width/media_height→width/height, dropping BMP), which aiosendspin hadn't caught up to. aiosendspin was rejecting everyclient/hellooutright as malformed (media_width/media_heightare required fields aiosendspin's model still expected, and sendspin-jvm no longer sends them) — that's why every scenario failed, not just the artwork one. Fixed this repo'saiosendspin_client.py/aiosendspin_server.pyadapters, which also constructedArtworkChannelwith the old field names and referenced the now-removedPictureFormat.BMP. The actual aiosendspin-side fix is Sendspin/aiosendspin#343 — this PR depends on that one merging (or at least landing onaiosendspin'smain) for the full matrix to go green, since the harness resolvesaiosendspinas a sibling git checkout.Test plan
python -m conformance.build(build_selected_adapters({"sendspin-jvm-client"})) succeedsimport conformance.adapters.aiosendspin_serversmoke-test import passesconformance run --from aiosendspin --to sendspin-jvm) — needs Sendspin/aiosendspin#343 merged first; verify via this PR's CI once that lands🤖 Generated with Claude Code