Skip to content

feat: add support for Mopeka Standard "Check" sensors - #97

Open
kahombur wants to merge 1 commit into
Bluetooth-Devices:mainfrom
kahombur:add-standard-check-support
Open

feat: add support for Mopeka Standard "Check" sensors#97
kahombur wants to merge 1 commit into
Bluetooth-Devices:mainfrom
kahombur:add-standard-check-support

Conversation

@kahombur

@kahombur kahombur commented Aug 7, 2026

Copy link
Copy Markdown

What / why

The decode in mopeka-iot-ble currently supports only the Pro family (Nordic manufacturer 0x59, service 0xFEE5). The original Standard "Check" sensor (e.g. Mopeka 8015004) is a different device — a TI chip advertising service UUID 0xADA0 — and is silently dropped today.

Unlike the Pro (which computes the level on-device), the Standard sensor broadcasts raw ultrasonic echo data (12 time/amplitude pairs); the client must run peak-detection and the LPG speed-of-sound calculation. This ports that algorithm from ESPHome's mopeka_std_check.

Changes

  • Minimal diff: _start_update gains one guard that routes Standard adverts (service 0xADA0) to a new _update_std method. The existing Pro path is untouched.
  • _update_std emits the same keys as the Pro path where applicable: tank_level (mm), temperature, battery, battery_voltage, reading_quality, reading_quality_raw.
  • Supported Standard hardware ids: Standard (0x02), XL (0x03), Standard-Alt (0x44), eTrailer (0x46).
  • tests/test_standard.py: full decode, no-usable-echo, unsupported-id, and a Pro-path regression check.

Validation

Validated against real hardware — a Mopeka Standard Check sensor decodes to values matching the Mopeka Check phone app (20 lb tank reading, temperature, battery). Tests use constructed advertisements with hand-verified expected outputs.

Notes

  • Opened as a draft to let CI validate. Happy to adjust to conventions (e.g. convert the value-based tests to the full SensorUpdate snapshot style used in test_parser.py).
  • Left the package version bump to the maintainer's release process.
  • Companion Home Assistant core change (separate PR): add a 0xADA0 Bluetooth matcher to the mopeka integration manifest so these are discovered; the existing config flow + sensor descriptions already handle the emitted entities.

Credit: decoding algorithm ported from ESPHome (mopeka_std_check).

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.21429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.00%. Comparing base (5fb292a) to head (231c2fd).
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
src/mopeka_iot_ble/parser.py 98.21% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
+ Coverage   91.48%   94.00%   +2.51%     
==========================================
  Files           3        3              
  Lines          94      150      +56     
  Branches        5       12       +7     
==========================================
+ Hits           86      141      +55     
  Misses          5        5              
- Partials        3        4       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The Standard sensor (service 0xADA0) is a different protocol from the Pro
line: it broadcasts raw ultrasonic echo data, so the client runs the
peak-detection + LPG speed-of-sound math itself. This ports ESPHome's
mopeka_std_check algorithm.

_start_update dispatches Standard adverts to a new _update_std; the existing
Pro path is unchanged. Emits tank_level (mm), temperature, battery,
battery_voltage, and reading_quality. Validated against real hardware
(matches the Mopeka Check app).
@kahombur
kahombur force-pushed the add-standard-check-support branch from b9d72db to 71c43e3 Compare August 7, 2026 16:50
@kahombur
kahombur marked this pull request as ready for review August 7, 2026 16:59
@bdraco bdraco changed the title Add support for Mopeka Standard "Check" sensors feat: add support for Mopeka Standard "Check" sensors Aug 7, 2026
@bluetoothbot

Copy link
Copy Markdown

PR Review — feat: add support for Mopeka Standard "Check" sensors

Approve with nits. I verified the decode bit-for-bit against the ESPHome reference — the algorithm is faithful, and every test assertion is arithmetically correct. All findings are non-blocking.

Specific things done well:

  • The bit unpacking is exactly right. I fetched mopeka_std_check.h and checked the packed-bitfield layout by hand: time_0..value_3 occupy bits 0-39 of each 5-byte block, and int.from_bytes(..., "little") with >> 10*m / >> 10*m+5 reproduces GCC's little-endian packed allocation precisely. raw_temp really is 6 bits, so & 0x3F (vs. the Pro path's & 0x7F) is correct, not a copy-paste slip.
  • The peak-detection loop matches the reference including its non-obvious partmeasurement_time = 0 sits inside if value != 0, exactly as ESPHome has it, and the validity gate number_usable >= 1 and best_value >= 2 and best_time >= 2 is the exact inverse of ESPHome's rejection condition. Getting the reset placement right is the thing most ports of this get wrong.
  • The claim in the code comment that ESPHome doesn't check the company id is accurate. ESPHome defines MANUFACTURER_ID = 0x000D and then never references it — I read the whole parse_device to confirm.
  • Emitting None for tank level on poor quality is better than the reference, which publishes a misleading 0. It also matches this repo's existing Pro-path convention.
  • The Pro regression test is genuinely load-bearing — I hand-computed it (model 0x0C, temp 20°C, tank_level 16 → 6mm) and it checks out, so the routing guard really is proven not to disturb the existing path.

What I could not verify: no environment with bluetooth_sensor_state_data installed was available, so I could not execute pytest. I simulated the decode standalone instead and reproduced all six asserted values in test_standard_check_decodes plus both in the no-echo case.

Key issues (all suggestion, none block merge):

  • MOPEKA_STD_PACKAGE_LEN = 19 is the struct size, not the 23-byte wire length ESPHome validates — so no test uses the payload shape real hardware sends.
  • The medium_type constructor argument is silently ignored on the Standard path while the HA config flow still collects it.
  • reading_quality_raw is 0–31 here but 0–3 on the Pro path, under the same entity key.
  • The raw_temp == 0 → -40°C sentinel is untested, and it swings the computed distance by >50% — almost certainly Codecov's one partial branch.
  • sync_pressed (bit 7 of data[3]) is masked away rather than emitted as the Pro path's button_pressed.
  • Distance differs from ESPHome by ~1mm (34 vs 35 on the test vector) because this port keeps float temperature and rounds; worth a comment given the PR cites phone-app agreement.
  • The comment on tests/test_standard.py:40 says value_0=15; it actually decodes to 31 (which the test then asserts).

On your open question about test style: converting to the full SensorUpdate snapshot form used in test_parser.py would fit the repo's convention better and would catch entity-description drift (names, device class, units) that the current value-only _values() helper cannot see. Worth doing, but not a merge condition.


🟢 Suggestions

1. Advert length constant is the struct size (19), not the wire length (23) ESPHome validates
src/mopeka_iot_ble/parser.py:61-62

I fetched the reference implementation to check this. ESPHome pins the advertisement payload at an exact length:

static const uint8_t MANUFACTURER_DATA_LENGTH = 23;
...
if (manu_data.data.size() != MANUFACTURER_DATA_LENGTH) { ... return false; }

19 is the size of mopeka_std_package (3 bytes + one 6/1/1 bitfield byte + 3 × 40-bit value blocks), which is what ESPHome reads, not what the sensor sends. Real hardware emits 23 bytes; the last 4 are simply not consumed by the struct cast.

Why it matters: the comment # 19-byte packed manufacturer payload will mislead the next maintainer into thinking the wire format is 19 bytes, and every test in test_standard.py builds a 19-byte advert — so nothing in the suite exercises the shape a real device actually transmits. Combined with >=, a truncated or unrelated 23-byte-class advert that happens to be ≥19 bytes and carries a matching data[1] & 0xCF will decode as a Mopeka Standard.

Suggested fix: keep the decode offsets as-is (they are correct), but reword the comment to distinguish the two numbers, and consider matching ESPHome's strictness:

# Advertised manufacturer payload is 23 bytes; the first 19 are the packed
# mopeka_std_package struct (the only part we decode).
MOPEKA_STD_ADV_LEN = 23

If you prefer to stay lenient, at minimum make one test use a 23-byte payload so the real wire format is covered.

# 19-byte packed manufacturer payload.
MOPEKA_STD_PACKAGE_LEN = 19
2. `medium_type` constructor argument is silently ignored on the Standard path
src/mopeka_iot_ble/parser.py:291

MopekaIOTBluetoothDeviceData.__init__ takes a medium_type and the Pro path threads it into tank_level_and_temp_to_mm(..., self._medium_type) (line 177). _update_std never reads self._medium_type — it always uses the module constant MOPEKA_PROPANE_BUTANE_MIX = 1.0.

Why it matters: the PR description states the existing Home Assistant config flow already handles these entities. That flow collects a medium type. A user who selects e.g. FRESH_WATER for a Standard sensor gets propane speed-of-sound math (~700 m/s vs ~1480 m/s for water) with no warning — a silently wrong distance, not an obvious failure.

I'd keep this non-blocking because the Standard "Check" is LPG-only hardware (the water variant is a Pro Check H2O) and ESPHome likewise only exposes a propane/butane mix for this component — so ignoring the setting is defensible. But it should be explicit rather than implicit.

Suggested fix: log it, or state it in the _update_std docstring:

if self._medium_type is not MediumType.PROPANE:
    _LOGGER.debug(
        "Standard Check is LPG-only; ignoring medium_type %s", self._medium_type
    )
distance_mm = round(std_speed_of_sound(temp_celsius) * best_time / 100.0)
3. `reading_quality_raw` has a different value domain (0–31) than the Pro path (0–3) under the same key
src/mopeka_iot_ble/parser.py:313-317

The Pro path emits reading_quality_raw as a 2-bit value (data[4] >> 6, range 0–3), and the trailing comment at line 202 documents the downstream rendering as (3-reading_quality) * "★" + (reading_quality * "⭐") — i.e. consumers may reasonably assume a 0–3 star scale.

_update_std emits the same key with best_value, a 5-bit amplitude in range 0–31.

Why it matters: the PR description promises the Standard path "emits the same keys as the Pro path where applicable." The key is the same but the scale is not, so anything reusing the Pro star-rendering logic against this key will be wrong by ~10×. The derived reading_quality percentage is correctly normalised (/ 31 vs / 3), so only the raw key is affected.

Since entities are created per-device, this is unlikely to break a live install today. Suggested fix: add a short comment next to the Standard reading_quality_raw noting the 0–31 domain, so the divergence is discoverable.

self.update_sensor(
    "reading_quality_raw",
    None,
    best_value,
    None,
    "Reading quality raw",
)
4. The `raw_temp == 0` → -40°C sentinel branch is untested
src/mopeka_iot_ble/parser.py:259

temp_celsius = -40.0 if raw_temp == 0 else ... faithfully mirrors ESPHome's parse_temperature_, but no test in test_standard.py sets data[3] & 0x3F == 0 — all three Standard tests use 0x2A. This is very likely the single partial branch Codecov flagged (98.21% patch coverage).

Why it matters: the sentinel is not merely a reported-temperature edge case. temp_celsius feeds std_speed_of_sound(), and -40°C yields ~1063 m/s versus ~697 m/s at 30°C — a >50% swing in the computed tank_level. A regression that dropped the sentinel would silently produce plausible-looking but badly wrong distances rather than an obvious crash.

Suggested fix: add a case asserting both halves of the consequence:

def test_standard_check_temp_sentinel() -> None:
    """raw_temp == 0 reports -40 C and feeds the distance calc."""
    data = bytes([0x00, 0x02, 0xE0, 0x00, 0xE4, 0x03] + [0] * 13)
    values = _values(
        MopekaIOTBluetoothDeviceData().update(
            _service_info({13: data}, STD_SERVICE_UUID)
        )
    )
    assert values["temperature"] == -40.0
temp_celsius = -40.0 if raw_temp == 0 else (raw_temp - 25.0) * 1.776964
5. `sync_pressed` bit is decoded away but never emitted, unlike the Pro path's `button_pressed`
src/mopeka_iot_ble/parser.py:258

data[3] & 0x3F masks off the top two bits, which the ESPHome struct names explicitly:

u_int8_t raw_temp : 6;
bool slow_update_rate : 1;   // data[3] bit 6
bool sync_pressed : 1;       // data[3] bit 7

sync_pressed is the same physical button the Pro path already publishes as the button_pressed occupancy binary sensor (line 161-166). The PR description says the Standard path emits "the same keys as the Pro path where applicable" — this one is applicable and is being discarded.

Why it matters: the Home Assistant mopeka integration already has a button_pressed binary-sensor description, so surfacing it here is nearly free and gives Standard users parity with Pro users for the pairing/sync workflow. Its absence just means the entity silently never appears.

Suggested fix:

self.update_predefined_binary_sensor(
    BinarySensorDeviceClass.OCCUPANCY,
    bool(data[3] & 0x80),
    key="button_pressed",
    name="Button pressed",
)

The mask itself is correct — I verified raw_temp is 6 bits wide against the ESPHome header, so & 0x3F (rather than the Pro path's & 0x7F) is right.

raw_temp = data[3] & 0x3F
6. Test comment states the wrong decoded amplitude (15 vs actual 31)
tests/test_standard.py:40

The comment is the only explanation for the magic bytes, and it doesn't match what they decode to.

data[4:9] = E4 03 00 00 00 → little-endian 0x03E4 = 996:

  • time_0 = (996 & 0x1F) + 1 = 4 + 1 = 5 (the raw nibble is 4, but the stored time is 5 — ESPHome adds 1)
  • value_0 = (996 >> 5) & 0x1F = 31, not 15

31 is what the test itself then asserts (reading_quality_raw == 31), so the code and assertions are right — only the comment is wrong.

Why it matters: this file's docstring presents these as "hand-verified expected outputs." A comment that contradicts the assertion undermines exactly that claim and will send the next person debugging a decode change down the wrong path.

Suggested fix:

# ... one strong echo (raw time_0=4 -> 5 ticks, value_0=31) in the
# first measurement block.
# (time_0=4, value_0=15) in the first measurement block.

Checklist

  • Decode algorithm faithful to cited reference (ESPHome mopeka_std_check)
  • Bit offsets and field masks verified against reference struct
  • Existing Pro path unaffected (regression test verified by hand)
  • No hardcoded secrets, injection, or unsafe deserialization
  • No index-out-of-range risk (length guard short-circuits before data[1])
  • Advertisement length validation matches real wire format — suggestion #1
  • Constructor configuration honored on all decode paths — suggestion #2
  • Shared entity keys have consistent value domains across device families — suggestion #3
  • Sentinel and edge-case branches covered by tests — suggestion #4, suggestion #1
  • PR description matches delivered scope (no scope creep) — suggestion #5, suggestion #3
  • Test comments consistent with actual decoded values — suggestion #6
  • No bare except / swallowed errors / resource leaks
  • No mutable default args, is vs == misuse, or eval/exec

Automated review by Kōan (Claude) HEAD=71c43e3 6 min 5s

@bluetoothbot bluetoothbot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

No blocking issues found — ready to merge.

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.

2 participants