Fix lsl_get_channel_bytes to return 0 for cft_string as documented (extends #261)#281
Merged
Merged
Conversation
With channel_bytes() now returning 0 for cft_string, the server no longer volunteers an endian conversion for string streams (value size 0), so it advertises its own host byte order instead of echoing the client's. The string length-prefixes are byte-swapped on the receiving side instead. Assert the host byte order here; the float case still covers actual binary endian conversion. 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
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.
Extends #261 by @myd7349.
lsl_get_channel_bytesis documented to return 0 for the string type (cft_string), and the wire protocol already assumes this —data_receiver.cppliterally annotates theValue-Sizeheader with// 0 for strings. In practice it was returning the (platform-dependent)sizeof(std::string)instead. This restores the documented contract.Why the abandoned endian test failure is expected, not a regression
@myd7349 paused this PR because the
tcpserverstringendiantest failed. I traced it: the server's defaultclient_value_sizecomes fromchannel_bytes(). With the fix it's 0 for strings, so the server no longer volunteers an endian conversion (value_size > 1is false) and advertises its own host byte order instead of echoing the client's4321.This is not a data-path regression:
format_sizes[]array insample.h(unchanged here), so string length-prefixes are still byte-swapped when needed.Byte-Orderheader (data_receiver.cpp). The fix only shifts which side swaps the length-prefixes; all cross-endian / cross-version combinations remain correct.Changes
channel_bytes().tcpserverstringendiansubtest to assert the server advertises its host byte order (no reversal for strings). The float case (tcpserver_float) still covers actual binary endian conversion.Verified locally on macOS:
lsl_test_internal(310 assertions) andlsl_test_exported(724 assertions) all pass.Closes #261.
🤖 Generated with Claude Code