Skip to content

fix: serialize battery_state as string to avoid precision loss in wasm bindings - #45

Open
JarrenMorris wants to merge 1 commit into
lvauvillier:masterfrom
JarrenMorris:fix/battery-bitfield
Open

fix: serialize battery_state as string to avoid precision loss in wasm bindings#45
JarrenMorris wants to merge 1 commit into
lvauvillier:masterfrom
JarrenMorris:fix/battery-bitfield

Conversation

@JarrenMorris

Copy link
Copy Markdown

Summary

Background: truncated aircraftSn

The log format stores details.aircraftSn in a fixed 16-byte slot. Newer
aircraft (e.g. Mavic 3 Enterprise) have 20-character serials, so the last
4 characters get truncated at the source — details.aircraftSn comes back
4 characters short of the real serial.

To work around this, we added a TS-side enrichment step that pulls the full,
untruncated serial from the length-prefixed ComponentSerial record instead,
via parser.records():

    // records() can throw where frames() doesn't (it serializes every raw record
    // field across the WASM boundary), so never let this enrichment kill a parse.
    try {
      const aircraftComponentSn = parser
        .records(keychains as KeychainFeaturePoint[][] | undefined)
        .find(
          (r): r is Extract<LogRecord, { type: "ComponentSerial" }> =>
            r.type === "ComponentSerial" && r.content.componentType === "Aircraft",
        )?.content.serial
      if (aircraftComponentSn) {
        details.aircraftSn = aircraftComponentSn
      }
    } catch (err) {
      console.warn(
        `ComponentSerial lookup failed, keeping details.aircraftSn as-is: ${err instanceof Error ? err.message : err}`,
      )
    }

The bug this workaround ran into

records() serializes every raw record field across the WASM boundary,
including SmartBatteryDynamic::battery_state — a bitfield that can exceed
2^53, the largest integer JS can represent exactly as a number. When that
value was out of range, serde-wasm-bindgen failed to serialize it, records()
threw, the catch above swallowed the error, and details.aircraftSn silently
fell back to the truncated 16-byte value — defeating the workaround on exactly
the logs it was meant to fix.

Fix

Map battery_state to a String at parse time
(#[br(map = |x: u64| x.to_string())]) so the full 64-bit value survives the
trip through JS without precision loss, and records() no longer throws for
these logs.

…rsing step in the battery where a number was too big
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant