BLE: deliver advertisement payloads with scan results#239
Conversation
Scan results carried only name, address and RSSI; the service data and manufacturer data that devices broadcast precisely to avoid connections were dropped at every bridge. This blocked identifying devices that advertise only service data (KBeacons' 0x2080) or manufacturer data (iBeacon), reading broadcast state like battery bytes, and recovering MACs on iOS. - New pure module Hatter.BleAdvertisement (re-exported from Hatter.Ble): parses raw AD structures into service data entries (keyed by the full lowercase 128-bit UUID text, 16/32-bit UUIDs expanded with the Bluetooth base UUID) and manufacturer data entries (keyed by the 16-bit company id), with serviceDataForUuid for case-insensitive lookup. Malformed air data degrades to "no payload" instead of failing: zero padding terminates, truncated tails are dropped. - BleScanResult gains bsrAdvertisement; the scan callback chain (Java/ObjC -> C -> haskellOnBleScanResult -> dispatchBleScanResult) gains a bytes+length parameter, mirroring the notification path. - Android passes ScanRecord.getBytes() through unchanged. iOS re-encodes CoreBluetooth's parsed advertisementData dictionary into the same AD structure format (CoreBluetooth never exposes the raw bytes), so Haskell has a single decoding path for both platforms. - The virtual test peripheral now broadcasts a 0xFEED service data entry and ble.sh asserts its payload arrives parsed in Haskell; BleDemoMain logs advertisement service data as decimal byte lists like the GATT logs. Eight new unit tests cover the parser and the dispatch path. Closes jappeace#238. Prompt: add these as issues to hatter if you feel it would be a nice addition, I maintain that as well / I suppose we've to address 238 first to even support this tool Co-Authored-By: Claude Fable 5 <[email protected]>
Co-Authored-By: Claude Fable 5 <[email protected]>
Review findings: the service-data UUID flowed through four signatures as bare Text while the sibling module newtypes every UUID string, and the same file already newtypes ManufacturerId for the other key. Move NormalizedBleUuid (with its Decision comment) into Hatter.BleAdvertisement, the import direction Hatter.Ble already has, and key advServiceData with it; the parser constructs it lowercase by construction. Also rename the abbreviated "adv" parameters to "advertisement" per the full-names style rule. Co-Authored-By: Claude Fable 5 <[email protected]>
| parseBleAdvertisement :: ByteString -> BleAdvertisement | ||
| parseBleAdvertisement bytes = | ||
| case BS.uncons bytes of | ||
| Nothing -> emptyBleAdvertisement |
There was a problem hiding this comment.
this should report the failure condition, every branch should have a unique one explaining what it is as a parseError
There was a problem hiding this comment.
Done in 99d34d8: each branch now has its own constructor stating what failed, why and where: AdStructureTruncated offset declared remaining, ServiceDataUuidTruncated offset adType uuidWidth actual, ManufacturerDataTooShort offset actual. The zero-length byte stays a clean terminator (it is Android's buffer padding, not a defect), and unknown AD types stay non-failures since names/flags/UUID lists are legitimate structures this module deliberately does not surface. Decision comment links your failing-in-haskell post.
| -- come straight off the air from arbitrary third-party devices, so a | ||
| -- garbled advertisement must degrade to "no payload", never take the | ||
| -- app down or suppress the scan result carrying it. | ||
| parseBleAdvertisement :: ByteString -> BleAdvertisement |
There was a problem hiding this comment.
typesig should be
parseBleAdvertisement :: ByteString -> Either ParseErrors BleAdvertisement
There was a problem hiding this comment.
Done in 99d34d8: parseBleAdvertisement :: ByteString -> Either AdvertisementParseErrors BleAdvertisement, with AdvertisementParseErrors a NonEmpty of every defect found (concrete types, no mtl, per your follow-up). The scan dispatch logs the defects with the sending device's address and still delivers the result, and bsrAdvertisement carries the full Either so apps can surface them too.
| Just (adType, payload) -> addAdStructure adType payload parsedRest | ||
|
|
||
| -- | Fold one AD structure into the advertisement parsed from the | ||
| -- bytes after it. Only the payload-bearing types are kept: service |
There was a problem hiding this comment.
put the failure conditions in the type signature
There was a problem hiding this comment.
Done in 99d34d8: addAdStructure/addServiceData/addManufacturerData all return Either AdvertisementParseError BleAdvertisement now.
|
|
||
| -- | Format 16 big-endian bytes as @xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx@. | ||
| formatUuid128 :: ByteString -> Text | ||
| formatUuid128 bigEndian = |
There was a problem hiding this comment.
Done in 99d34d8 using uuid-types (the type-only package): UUID.fromWords is total and UUID.toText gives the canonical lowercase form, killing the hand-rolled hex. The full uuid package pulls network-info (OS ioctls) which would fight the Android cross build, so I stopped at uuid-types; its non-boot transitives (hashable, random, splitmix) are added to the cross-deps/ios-deps collection lists. The whole-API typed-UUID migration stays tracked in #240.
…-types Review findings on jappeace#239: the parser silently dropped malformed structures (truncated tails, service data too short for its UUID, manufacturer data without a company id), and the UUID text rendering was hand-rolled. Per https://jappie.me/failing-in-haskell.html we want to know what fails, why and where, with concrete types rather than mtl: - parseBleAdvertisement :: ByteString -> Either AdvertisementParseErrors BleAdvertisement. Every malformed branch has its own constructor carrying the byte offset of the offending structure plus the sizes involved (AdStructureTruncated, ServiceDataUuidTruncated, ManufacturerDataTooShort); all defects in one advertisement are accumulated as a NonEmpty. Unknown AD types remain non-failures (names, flags, UUID lists are legitimate structures this module does not surface). The zero padding byte remains a clean terminator. - The helpers carry the failure in their signatures too (Either AdvertisementParseError). - bsrAdvertisement is now the full Either: the scan dispatch logs defects loudly with the sending device's address and still delivers the result, and applications can surface them as well. - UUID rendering goes through uuid-types (UUID.fromWords is total, UUID.toText is canonical lowercase), replacing the hand-rolled hex formatting; the Bluetooth base-UUID aliasing keeps its three word constants. uuid-types and its non-boot transitive deps (hashable, random, splitmix) are added to the cross-build collection lists. Prompt: address my review comments, refer to my blogpost failing in haskell. you've added silent failures, that's bad, we want to know what fails, why and where / I use mtl style in the blogpost, that's not a good idea, use concrete types instead if you can Co-Authored-By: Claude Fable 5 <[email protected]>
The offset threaded through four parser functions and stored in all three error constructors was a bare Int, and sat next to the equally bare uuidWidth Int in addServiceData where the two could transpose without a type error. AdStructureOffset makes that mistake unrepresentable. Co-Authored-By: Claude Fable 5 <[email protected]>
A package can reach the collector twice, once through a consumer's resolved deps and once through hatterOwnDeps (hashable did, as a dep of both unordered-containers and the new uuid-types), and the second cp of the same .conf into the read-only output failed with Permission denied. Guard the .conf copy like the .a copies already are. This is the dedup fix kbeacon's consumer-cabal2nix Decision comment deferred to hatter. Co-Authored-By: Claude Fable 5 <[email protected]>
The job's VM builds anything missing from nix-cache.jappie.me under TCG emulation on every run (its store is ephemeral, so the host-side actions cache cannot help), and the uuid-types closure from PR jappeace#239 hit the 120-minute timeout at exactly 2h00m (run 29272233710, step cancelled 12 seconds after start+120min). Raise the budget so an uncached closure can complete; cached runs stay at ~45 minutes. Co-Authored-By: Claude Fable 5 <[email protected]>
The ios build itself passed; the combined simulator test step was cancelled at the job timeout, the known flake class from issue jappeace#235 (same remedy as 9dcf105). Co-Authored-By: Claude Fable 5 <[email protected]>
| AdStructureTruncated | ||
| AdStructureOffset -- ^ Where the structure starts. | ||
| Int -- ^ Length the structure declares. | ||
| Int -- ^ Bytes actually remaining after the length byte. |
There was a problem hiding this comment.
actually for these it maybe better instead of a newtype for AdStructureOffset, you create a new data type for the two ints and StructureOffset. So that every error sum constructor get's it's own record, allowing us record access to the fields
There was a problem hiding this comment.
Done in 69e1ec3: each error constructor now wraps its own record (AdStructureTruncation, ServiceDataTruncation, ManufacturerDataTruncation) bundling the AdStructureOffset with the sizes, so every field has a total record accessor (single-constructor records dodge -Wpartial-fields).
| Word8.toWord32 (BS.index bytes offset) `shiftL` 24 | ||
| .|. Word8.toWord32 (BS.index bytes (offset + 1)) `shiftL` 16 | ||
| .|. Word8.toWord32 (BS.index bytes (offset + 2)) `shiftL` 8 | ||
| .|. Word8.toWord32 (BS.index bytes (offset + 3)) |
There was a problem hiding this comment.
BS.index is partial, use index maybe and push out the maybe. in the typesignature.
There was a problem hiding this comment.
Done in 69e1ec3: no BS.index left in the module. word32BigEndianAt chains BS.indexMaybe and returns Maybe Word32; normalizedUuidFromLittleEndian pushes it out as Maybe NormalizedBleUuid (Nothing unless the slice is exactly 2/4/16 bytes), and addServiceData consumes that Maybe as its truncation check, so the length validation lives in exactly one place. addManufacturerData reads its two bytes via indexMaybe with all four cases written out, and the structure walker peels the type byte with BS.uncons.
There was a problem hiding this comment.
Addendum in c3445d7: the first indexMaybe version had a truncation hole (judging the slice by its own length let a 0x21 structure truncated to two bytes pass as a 16-bit UUID, and [0x80, 0x20] would read as 0x2080 itself). normalizedUuidFromLittleEndian now takes the declared width and returns Nothing unless the slice has exactly that width; regression tests cover 0x21 truncated to 2/4 bytes and 0x20 truncated to 2.
Review round 2 on jappeace#239: - Every AdvertisementParseError constructor now wraps its own record (AdStructureTruncation, ServiceDataTruncation, ManufacturerDataTruncation) bundling the offset with the sizes, so the fields have total record accessors instead of positional Ints. - All BS.index uses are gone: word32BigEndianAt chains BS.indexMaybe and pushes Maybe into its signature, normalizedUuidFromLittleEndian returns Maybe (Nothing unless the slice is exactly 2, 4 or 16 bytes), and addServiceData consumes that Maybe as its truncation check, leaving one place that decides what a valid UUID slice is. addManufacturerData reads its two company-id bytes via indexMaybe with all four cases written out. The structure walker peels the type byte with BS.uncons on the already-split structure instead of an index. Prompt: address the review comments in hatter Co-Authored-By: Claude Fable 5 <[email protected]>
The indexMaybe refactor (69e1ec3) introduced a truncation-detection hole: addServiceData split the payload at the declared width and let normalizedUuidFromLittleEndian judge the slice by its own length, so a 0x21 structure (16-byte UUID) truncated to two bytes passed as a valid 16-bit UUID, and [0x80, 0x20] would read as 0x2080, the exact KKM identity UUID. The builder now takes the declared width and returns Nothing unless the slice has exactly that width, restoring the parent commit's behaviour with the single-decision-point shape kept. Regression tests cover 0x21 truncated to 2 and 4 bytes and 0x20 truncated to 2. Co-Authored-By: Claude Fable 5 <[email protected]>
| (word32BigEndianAt | ||
| (BS.replicate (4 - declaredWidth) 0x00 <> bigEndian) 0) | ||
| else Nothing | ||
| in fmap (NormalizedBleUuid . UUID.toText) uuid |
There was a problem hiding this comment.
this is silly, can't we just return the UUID type instead of converting it back to "normalized" text?
There was a problem hiding this comment.
Done in 119e58c: advServiceData is keyed by uuid-types' binary UUID (case differences unrepresentable by construction), uuidFromLittleEndian returns Maybe UUID, and serviceDataForUuid takes the UUID value; constants build with the total fromWords. Text now only appears at the logging edge. NormalizedBleUuid moved back to Hatter.Ble with the characteristic keys, its remaining users. This is also the first slice of #240.
Review: converting the parsed UUID back to normalized text was silly. advServiceData is keyed by uuid-types' binary UUID now (comparisons cannot be bitten by platform case differences, by construction), uuidFromLittleEndian returns Maybe UUID, and serviceDataForUuid takes the UUID value; constants build with the total UUID.fromWords. Text conversion happens only at the logging edge (UUID.toText in the demo). NormalizedBleUuid returns to Hatter.Ble, where its remaining users (the characteristic keys) live. Prompt: one more review comment Co-Authored-By: Claude Fable 5 <[email protected]>
| parseBleAdvertisement bytes = | ||
| case parseAdStructuresFrom (AdStructureOffset 0) bytes of | ||
| (advertisement, []) -> Right advertisement | ||
| (_, firstDefect : moreDefects) -> |
There was a problem hiding this comment.
I guess we can expose the partially parsed advertisement on the right branch as well so you get Either BleAdvertiseWithErrors BleAdvertisement
data BleAdvertismentWIthError = BleWithErrors { advertisement :: BleAdvertistement, parseErrors :: AdvertisementParseErrors }
There was a problem hiding this comment.
Judged it worth doing, done in 079dadf. The comment makes sense and the partial is usually NOT empty: AD structures are independent TLVs, so a mid-stream defect skips only its own structure (everything before and after survives) and a truncation keeps everything before it; the salvage is empty only when the FIRST structure is truncated or every structure is defective. Since the link layer's CRC already dropped radio corruption, a defect here is a firmware quirk in one structure and the rest is trustworthy, so discarding it was wrong (a KBeacon with one garbled trailing structure would have lost its valid 0x2080 identity data). Implemented as your Either BleAdvertisementWithErrors BleAdvertisement with the record; tests pin a filled-salvage case (truncated tail keeps the manufacturer data before it) and an empty-salvage case (all structures defective).
Review: parseBleAdvertisement returns Either BleAdvertisementWithErrors BleAdvertisement now, the Left a record of the partial advertisement plus the NonEmpty defects. AD structures are independent TLVs, so the partial holds every structure that still parsed (a mid-stream defect skips only itself; a truncation keeps everything before it) and is empty only when the first structure is truncated or all are defective. The link layer's CRC already dropped radio corruption, so defects here are firmware quirks in single structures and the well-formed remainder is trustworthy: a beacon with one garbled structure must not lose its valid service data. Tests pin both the filled-salvage and empty-salvage cases. Prompt: one more review comment for hatter. I'm not sure if it makes sense though, like if we get an error does it make sense to return the partial result, will this always be an empty ble advertisemetn or will it be filled with something? I don't know, you make the judgement if the review comment even make sense Co-Authored-By: Claude Fable 5 <[email protected]>
… job Also serves as the diagnostic for the kbeacon x86_64 emulator startup SIGSEGV: the android job's x86_64 emulator tests never ran on 079dadf. Co-Authored-By: Claude Fable 5 <[email protected]>
Review finding: run 29334286560 passed (44m17s, on jappeace#243) and was miscited as a timeout casualty. The actual prior instance of the build-passes-simulator-test-cancelled fingerprint is run 29281533228 on jappeace#239, alongside this PR's run 29344658446 and the historical mid-compile timeout 9dcf105. Co-Authored-By: Claude Fable 5 <[email protected]>
2 participants
Closes #238. First consumer: jappeace/kbeacon-ota-tool (identify KBeacons by their 0x2080 service data like KKM's own library, battery-at-scan, renamed beacons on iOS).
What
Hatter.BleAdvertisement(re-exported fromHatter.Ble): parses raw AD structures (length : type : payload) into service data entries, keyed by full lowercase 128-bit UUID text (16/32-bit UUIDs expanded with the Bluetooth base UUID, matching the existing textual convention), and manufacturer data entries keyed by the 16-bit company id.serviceDataForUuiddoes case-insensitive lookup.BleScanResultgainsbsrAdvertisement; the whole scan callback chain (Java/ObjC → C →haskellOnBleScanResult→dispatchBleScanResult) gains a bytes+length parameter, mirroring the existing notification-payload path.ScanRecord.getBytes()through unchanged. iOS re-encodes CoreBluetooth's parsedadvertisementDatadictionary back into the same AD structure format (CoreBluetooth never exposes the raw bytes), so Haskell has a single decoding path for both platforms.Tests
0xFEEDservice data entry (payload2A 63) andble.shasserts the parsed bytes arrive in Haskell;BleDemoMainlogs advertisement service data as decimal byte lists like the GATT logs.cabal test(nix-build is blocked in my sandbox); NDK/ObjC sides mirror the existingonBleNotificationbyte-array plumbing and are CI-verified.🤖 Generated with Claude Code