Translate outbound config packets for legacy-protocol devices#192
Open
cboulay wants to merge 1 commit into
Open
Translate outbound config packets for legacy-protocol devices#192cboulay wants to merge 1 commit into
cboulay wants to merge 1 commit into
Conversation
Two latent bugs prevented protocol 4.0 / 3.11 devices from being configured (found while investigating a firmware 7.7.0 connect failure): 1. Protocol wrappers (DeviceSession_311/400/410) delegate the high-level config helpers (setChannelConfig, setSystemRunLevelSync, setSampleGroup, …) to the wrapped DeviceSession, whose sendPacket() did NOT translate. So those helpers sent current (4.1+) wire-format packets to legacy devices, which could not parse them — the device never replied and the sync barrier timed out. Only the wrapper's own sendPacket() override translated, and the delegated helpers bypassed it. Centralize outbound translation in DeviceSession::sendPacket(), keyed by a send protocol the wrappers set via setSendProtocol() at construction. Wrappers now delegate sendPacket() to the wrapped session instead of each duplicating translation, so every send path — direct or via a delegated helper — is translated in one place. 2. cbPKT_HEADER_400 in packet_translator.h was not byte-packed, so sizeof was 24 not 16 (dlen landed at offset 12 instead of 11), corrupting every translated 4.0 packet. Add #pragma pack(1) and static_asserts pinning the 3.11 (8-byte) and 4.0 (16-byte) header sizes. Adds LegacyConfigHelper_TranslatesOutbound_311/400 regression tests that drive a *delegated* config helper and assert the on-wire legacy header layout (before the fix they arrived as current-format packets). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 latent bugs prevent protocol 4.0 and 3.11 devices from being configured. They're independent of the firmware-7.7.0 pacing fix (#191, released in v9.12.1) — that issue turned out to be UDP-buffer overrun on a 4.1 Hub, not translation. These fixes matter when actually talking to 4.0/3.11 hardware, where the config path currently sends unparseable packets.
Bug 1 — config helpers bypass protocol translation
The protocol wrappers (
DeviceSession_311/400/410) wrap aDeviceSessionby composition and delegate the high-level config helpers (setChannelConfig,setSystemRunLevelSync,setSampleGroup, …) straight to the wrapped session. Those helpers callsendPacket()internally — but on the wrappedDeviceSession, whosesendPacket()did not translate. Only the wrapper's ownsendPacket()override translated, and the delegated helpers never reached it. So on a legacy device every config packet went out in current (4.1+) wire format, the device couldn't parse it, and the sync barrier timed out.Fix: centralize outbound translation in
DeviceSession::sendPacket(), keyed by a send protocol the wrappers set viasetSendProtocol()at construction. Wrappers now delegatesendPacket()to the wrapped session instead of each duplicating translation, so every send path — direct or via a delegated helper — is translated in exactly one place. Removes the three per-wrappersendPacketoverrides.Bug 2 —
cbPKT_HEADER_400not byte-packedcbPKT_HEADER_400inpacket_translator.hlacked#pragma pack(1), sosizeofwas 24, not 16 (dlen landed at offset 12 instead of 11), corrupting every translated 4.0 packet. Added packing andstatic_asserts pinning the 3.11 (8-byte) and 4.0 (16-byte) header sizes. A unit test surfaced this.Testing
LegacyConfigHelper_TranslatesOutbound_311/_400tests drive a delegated config helper (notsendPacket()directly) and assert the on-wire legacy header layout — they fail before the fix (packets arrive current-format).cbdev_device_tests/cbproto_tests/cbsdk_testspass. (ConnectionParams_Predefined_NPlayfails onmastertoo — pre-existing, unrelated.)Rebased on
masterafter v9.12.1, so this is a clean, translation-only diff.🤖 Generated with Claude Code