flowsprenger / RxLifx-Swift

RxSwift based SDK for the LIFX Lan protocol
MIT License
10 stars 5 forks source link

Consistent UTF8 string streaming #7

Closed lightbow closed 6 years ago

lightbow commented 6 years ago

Previously, if the outgoing string were shorter than the max length, writeString would pad the start with space. Since the spec published online doesn't provide much guidance (other than length) for String fields in LIFX packets, I asked on the community forums (https://community.lifx.com/t/string-encoding/4126) where Daniel Hall (LIFX employee) replied "The LIFX app and most other apps use a NULL character (\0) terminated UTF-8 string."

Two changes to writeString: 1) Make sure we write out exactly "size" bytes by clipping the UTF8 array to that maximum 2) Instead of pre-padding space, write out the UTF8 bytes then fill the rest with \0

On the readString end of things, I'm not 100% sure that the existing code would handle all UTF8 cases properly since it makes individual characters before appending it to the new string. Instead: 1) Grab all of the bytes first 2) Guarantee it is null-terminated 3) Feed it into the swift string API designed to make sense of null-terminated UTF8 bytes

Together I believe these two changes will handle incoming and outgoing strings that behave better in the wild.

lightbow commented 6 years ago

I first noticed this when looking at some spurious light/group name changes in my model and seeing strings like "Track 1 (Piano)\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0" (when built up manually by character appending) or "\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}\u{14}LIFX BR30" (for the pre-padding writeString function) in the debugger. They'd show up fine in my UI, but the underlying String wasn't well-formed.

flowsprenger commented 6 years ago

Excellent good spotting! Thanks!