Fix NimBLEEddystoneTLM byte-order mismatch between getters and setters

[94xhn]

Problem

NimBLEEddystoneTLM::m_eddystoneData is meant to always hold Eddystone-TLM wire-format (big-endian) bytes: getData() returns it directly for the caller to memcpy straight into the BLE advertisement payload (see the BLE_EddystoneTLM_Beacon example in the sibling NimBLE-Arduino repo), and setData()/getData() already treat it purely as raw wire bytes with no conversion.

The getters (getVolt()/getTemp()) correctly un-swap on read via ENDIAN_CHANGE_U16. But the setters (setVolt/setTemp/setCount/setTime) store their host-order input with no swap, and the BeaconData default member initializers (volt{3300}, temp{23 * 256}) are host-order values, not pre-swapped wire-format values.

Effects of this mismatch:

• A freshly constructed NimBLEEddystoneTLM (before calling any setter) reports getVolt() == 58380 instead of the intended default 3300.
• setTemp(23 * 256); getTemp(); returns 23, not 5888 — the setter/getter round trip is broken for any value set locally (as opposed to arriving over the air via setData()).
• toString() never un-swaps the temperature field at all, and computes the displayed magnitude with unsigned division/modulo (m_eddystoneData.temp / 256) on a signed 8.8 fixed-point value, so negative temperatures print garbage.
• Since real usage (see the example referenced above) calls setVolt/setTemp and then transmits getData()'s raw bytes directly, the actual over-the-air TLM battery/temperature values are corrupted for any real device using this class today.

This is the same class of bug fixed for the float-based API prior to the int16_t-based refactor in #248, which reintroduced it.

The sibling NimBLEBeacon class already applies the swap in its setters (setMajor/setMinor/setManufacturerId) for the same reason — this fix brings NimBLEEddystoneTLM in line with that existing, working pattern.

Fix

• setVolt/setTemp/setCount/setTime now apply ENDIAN_CHANGE_U16/ENDIAN_CHANGE_U32 to their input before storing, matching what the getters already assume.
• BeaconData's default volt/temp member initializers are now the wire-format (byte-swapped) encoding of the intended defaults (3300 mV, 23.0°C), so a freshly constructed object reports correct values without calling a setter first.
• toString() now swaps the temperature field the same way it already swaps voltage, and computes the displayed magnitude from a signed value with explicit sign handling instead of unsigned arithmetic on the raw field.

No change to the over-the-air byte layout — setData()/getData() still operate on raw wire-format bytes exactly as before; only host-order local access via the individual setters/getters/toString() is fixed.

Testing

Verified with a standalone host-compiled (g++ -std=c++17) test extracting the exact struct/macro/getter/setter/toString logic:

• Default-constructed object: getVolt() == 3300, getTemp() == 5888, toString() prints "Battery Voltage 3300 mV" / "Temperature 23.0 C".
• setVolt(4000) / setTemp(5888) round-trip exactly.
• setTemp(-26) (-0.1015625°C) round-trips exactly and toString() prints "Temperature -0.10 C".
• setTemp(-1408) (-5.5°C) round-trips exactly and toString() prints "Temperature -5.50 C".
• The real over-the-air path via setData() with wire-format big-endian bytes (simulating a real radio) is unaffected and still correctly returns getVolt() == 3300, getTemp() == 5888.

Also syntax-checked the actual modified src/NimBLEEddystoneTLM.cpp/.h against the real header dependencies (with minimal local stubs for NimBLEUUID.h/NimBLELog.h/syscfg.h) — compiles cleanly with -Wall -Wextra.

Summary by CodeRabbit

• Bug Fixes
  • Fixed Eddystone TLM temperature string formatting to correctly handle fixed-point values, including negative temperatures and proper decimal output.
  • Ensured telemetry fields (voltage, temperature, packet count, and elapsed time) are stored in the correct wire-format endianness and interpreted consistently.
  • Updated default Eddystone TLM voltage and temperature values to match the correct big-endian wire format.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d1b91dce-80ea-441f-8f9d-6813b234e024

📥 Commits

Reviewing files that changed from the base of the PR and between 537fc32 and 8679dff.

📒 Files selected for processing (1)

• src/NimBLEEddystoneTLM.cpp

---

📝 Walkthrough

Walkthrough

Arrr, Eddystone TLM fields now store endian-adjusted wire values, while temperature rendering handles signed fixed-point data correctly.

Changes

Eddystone TLM endian handling

Layer / File(s)	Summary
Wire-format defaults and setter storage src/NimBLEEddystoneTLM.h, src/NimBLEEddystoneTLM.cpp	Default voltage and temperature values use big-endian constants, and all TLM setters apply matching endian conversions before storage.
Signed temperature formatting src/NimBLEEddystoneTLM.cpp	Temperature conversion swaps the fixed-point value and formats integer, fractional, and negative components explicitly.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

TLM bytes sail in endian tide,
Volt and temp keep wirewise stride.
Signed degrees now speak more clear,
Fractions dance without a sneer.
Yarrr, the beacon’s course is right!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Arrr, the title clearly matches the main byte-order fix between getters and setters.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/NimBLEEddystoneTLM.cpp`:
- Around line 104-109: Update the temperature format string in the value
formatting logic to render the fractional component with two digits, preserving
leading zeros for values below 10. Keep the existing sign, integer, and fraction
calculations unchanged.

In `@src/NimBLEEddystoneTLM.h`:
- Around line 40-41: Update the default member initializers for the voltage and
temperature fields in the Eddystone TLM data structure to pass the host-order
values (3300 mV and 23 × 256) through ENDIAN_CHANGE_U16 instead of using
pre-swapped literals. Ensure the header exposes the macro through the
appropriate existing include or definition, while preserving the current
wire-format behavior and getter conversions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 82247566-3e01-4d99-b92e-76705f2aba12

📥 Commits

Reviewing files that changed from the base of the PR and between 657583a and 537fc32.

📒 Files selected for processing (2)

• src/NimBLEEddystoneTLM.cpp
• src/NimBLEEddystoneTLM.h