Skip to content

fix(miot): 不再把缓存里的「上次已知值」当成设备的当前值端出去 - #485

Open
LeonJoeeee wants to merge 2 commits into
XiaoMi:mainfrom
LeonJoeeee:fix/stale-offline-device-reads
Open

fix(miot): 不再把缓存里的「上次已知值」当成设备的当前值端出去#485
LeonJoeeee wants to merge 2 commits into
XiaoMi:mainfrom
LeonJoeeee:fix/stale-offline-device-reads

Conversation

@LeonJoeeee

Copy link
Copy Markdown
Contributor

Closes #484

一句话根因

每一层存的都是「上次已知值」,端出去时却按「当前值」的形状呈现 —— 于是「没读到设备」和「设备就是这个值」在数据上不可区分。

事故:一条「主卧灯还亮着就播报睡眠提醒」的定时任务,在灯离线后 00:00~02:30 每 30 分钟朝卧室音箱播报一次,把睡着的人吵醒。灯的读数是 {"value": true, "code": 0},响应里没有一处承认存疑。当晚 6/6 条可解析的 agent trajectory 都先看到 offline、都复述了 offline,然后照样相信读数 —— 不是没判断,是判断了、判反了。

读设备状态在语义上是一次请求,不是一次查表:结果有三态 —— 开、关、读不到。今天的实现把第三态折叠进了前两态。

改了什么

1. datasource 从硬编码提成参数(backend/miot/src/miot/cloud.py)。此前 {"datasource": 1} 硬编码、无注释。实测:1 读云端缓存,对不可达设备返回最后已知值 + code 0;2 读真机,不可达设备由云端自己返回 -704042011。默认仍是 1,现有调用方零变化。

2. 定点查询读真机,全量冷查询读缓存(miot/service.py)。分档线复用已有的 user_specified:传了 iid 的调用方是在拿这个值当判据,它需要知道读数活不活;不传 iid 的是面板那种全量冷查询,WebUI 一屏 30+ 台同时打,那边的注释已写明会撞上 MiOT 约 10 QPS 限频。事故那条定时任务的日志行是 iid: prop.2.1,走这一档之后判据自然为假,任务一个字都不用改。

3. 幂等预检必须读真机(rule/runner.py)。这是本仓库内更隐蔽的一处:缓存的陈旧值 == 目标值 → 报「已在目标态」跳过,而这个 return 在所有 _write_action_ledger 之前 —— 断电后灯的缓存停在 on,一条「开灯」规则读到陈旧的 on 就永久跳过,台账一行都不写。ds=2 下云端返回 -704042011,已有的 code == 0 判据无需改动。

4. 透出云端的 updateTime(miot/service.py,CLI 换算 age_s)。云端每一行状态属性都带 updateTime,此前被丢弃 —— 于是一个 182 天前的读数与一个刚读到的读数在响应里完全同形。原样透传、不做换算;缺失给 null 而非 0(siid=1 静态元数据本就没有这个字段,给 0 会让调用方算出 56 年)。它是 as-of 注解,不是「设备还活着吗」 —— 实测在线设备的读数也可以很老(净饮机 25.1 小时)。

5. SKILL.md 收窄离线口径。「离线设备照常下命令」被限定到控制路径,并补了查询侧规则。文档是事故的近因,不改会把前面几条抵消掉。

6. ds=2 的批量悬崖(第二个提交)

读真机是全或无的,有约 4.2s 的服务端截止时间。一次请求的属性数超过该设备能在时限内答完的数量,整批返回 -704220043 且不带 updateTime —— 一台健康在线的设备被整台报成「读不到」,把响亮的假阳换成静默的假阴,正是这个 PR 要避免的方向。

悬崖位置因设备而异、但对同一台设备是确定的(实测,重复扫边界一致):

空调    14 个 ok(369ms) → 15 个整批失败(4374ms)
净化器  16 个 ok(857ms) → 20 个整批失败(4739ms)
洗衣机  16 个 ok(368ms) → 20 个整批失败(4179ms)

而端点接受逗号分隔的多个 iid(miot/router.py:316),这个家里 33 台设备中 11 台可读属性 ≥15 个(其中 8 台此刻在线)。现按 8 分批(最低那条悬崖的一半,常量里写清了来历),顺序发、结果按请求顺序拼接,调用方照旧按 iid 对号。同三台设备、整套可读属性,改前 / 改后:

次卧全景  71 属性   6038ms  0/71  →  1400ms  71/71
书房全景  43 属性   4138ms  0/43  →   889ms  40/43
空调      42 属性   4198ms  0/42  →  1758ms  42/42

分批比不分批更快 —— 注定失败的那次请求会把截止时间烧满。书房全景剩下的 3 个失败是云端逐属性给出的 -704220008,是整批失败此前一直盖住的真实答复。(71 属性那台相机整批失败时报的是 -704083036 而非 -704220043,同一个悬崖、不同的码。)

一批带着错误码回来不会影响其它批,该批的码原样返回 —— 只有云端真的返回过的码才算失败(#394),本地既不合成也不抹掉。传输层异常仍照常上抛:那意味着「没读到」,而一个悄悄少几行的响应正是这个 PR 要消灭的失败形态。

不并发的理由:对同一台设备开 6 路读回既加重设备负担,也更容易撞限频,而顺序分批本来就比「一次问完然后整批失败」更快。

代价(实测,真实账号)

ds=2 在线 15 属性 167249ms(ds=1 为 61ms)、离线 6790ms —— 不慢,云端凭在线注册表秒拒,不等超时;30 台并发 290ms vs ds=1 的 356ms,未触发限频。

本 PR 不做

以下两处是同一根因的其它面 —— 存的是「上次已知」,端出去时是「当前」。证据已经有了,但改动性质不同(涉及订阅 / 刷新调度),另开 PR 处理:

  1. miloco 自己那份「谁在线」的快照不刷新。 非相机设备既无周期刷新也不订阅上下线,实测最长冻结 6h48m;稳态下每天只在小米服务端 05:00 断线重连时更新一次。也正因为这样,本 PR 没有、也不该拿 online 当闸门:实测抓到过反例 —— 33 台逐台比对漂移 1 台,恰好是事故那盏灯,缓存说离线,而真机读回来它活着、亮着。拿它当判决会把一个吵闹的假阳换成一个静默的假阴。
  2. 相机唤醒态缓存,实测冻结 11h39m

WebUI 本次不需要改:它靠 real.tsif (!d.online) 屏蔽了陈旧值(#441 立的规矩),陈旧值虽然进了 Device.props,但今天没有渲染方。

测试

  • backend/miloco/tests/test_miot_read_through_batching.py(新):超过 chunk 的请求必须全部成功返回、顺序不变、单批失败不影响其它批且其错误码原样返回、ds=1 不被拆、不足一批不多花往返、chunk 值不得高于实测最低悬崖。(放在 miloco 的测试目录,是因为 backend/pyproject.tomlnorecursedirs = ["miot", ...] 会让 backend/miot/tests 完全不被收集。)
  • test_miot_service_lru.py / test_rule.py / cli/tests/test_commands.py:分档路由、updated_at 透传与缺失、云端码不被改写、不可达 ≠ 已在目标态、age_s 换算。
  • 全绿:backend 2807 passed、cli 557 passed;ruff check 在改动文件上干净。
  • 真机验证:上面所有数字都来自真实账号的只读探测(仅 miotspec/prop/get,未控制任何设备)。

开放问题

  • ds=2 会不会唤醒电池 / 休眠设备并耗电,没测(本机 6 台温湿度计全在线,无可测样本)。这是最可能在别的部署里出问题的地方。
  • updateTime 的精确云端语义无文档,我的解读(= 该值上次被写入的时刻)是反推的,所以只拿它做 as-of 注解,没让任何判断依赖它。

LeonJoeeee and others added 2 commits August 3, 2026 00:11
A cron in my home announces "it's late, time for bed" through the bedroom
speaker if the bedroom light is still on. On 2026-08-02 it fired every 30
minutes from midnight to 02:30 and woke the house.

The light had gone offline. `GET /api/miot/devices/{did}/status` answered with
its last known value and `code: 0`:

    device_list:  1112398686  床头灯  主卧  offline
    prop.2.1   -> {"value": true, "code": 0}

Nothing in that response admits doubt. The agent was not careless — six of six
recoverable trajectories that night show it running `device list` first, seeing
`offline`, saying so out loud, and then trusting the reading anyway:

    "The light is showing as \"offline\". Let me still try to check its status..."
    -> {"code":0,"message":"Device status retrieved successfully",
        "data":{"properties":[{"iid":"prop.2.1","value":true,"code":0}]}}
    "The light is ON (value=true). So I need to play a reminder..."

It was also following this repo's own documentation, whose offline guidance is
entirely about the control path and never mentions `properties[]`.

## Staleness, not offline-ness, is the axis

Measured against the live account, same instant, `datasource=1`:

    humidifier    offline   value:false  code:0   182.4 days old
    mosquito      offline   value:true   code:0    21.1 days
    air-con       offline   value:false  code:0     5.1 days
    water purif.  ONLINE    value:0      code:0    25.1 hours
    3-gang switch ONLINE    value:false  code:0    22.3 hours

Online devices serve day-old readings too, so gating on `online` would be both
wrong and unsafe. It is also unreliable: comparing all 33 devices against the
cloud twice, the cached flag disagreed on exactly the bedside lamp — cached
False, cloud True, and a read-through says the lamp is alive and on. Non-camera
devices have no periodic refresh and no online/offline subscription; the code
says so outright ("Non-camera devices are ignored"). An `online` gate would have
labelled a genuinely-lit lamp stale, trading a loud false positive for a silent
false negative.

Nor is it true that offline devices simply return `code: 0`. The response is a
mixture — the rice cooker returns cached values on some properties and the
cloud's own `-704042011` on others, in the same call. The lamp was silent only
because all three of its properties happened to be cache hits.

## What this changes

`datasource` was hardcoded to 1 in `cloud.py` with no comment. Measured: 1 reads
the cloud's cache, 2 reads through to the device — and at 2 the cloud itself
answers unreachable devices with `-704042011` rather than a stale value. Nothing
has to be fabricated locally, which also keeps XiaoMi#394's rule intact: only codes
the cloud actually returned count as failure.

It is now a parameter, defaulting to 1 so existing callers are unchanged, and
two call sites opt into 2:

- **Targeted queries** (`?iid=...`) read through; full cold queries keep the
  cache. The split reuses the existing `user_specified` flag rather than
  inventing a concept, and it lands exactly where it should: a caller naming a
  property is using it as a predicate, while the panel's 30-devices-at-once
  refresh must stay cheap — its own comment records hitting MiOT's ~10 QPS
  limit. The cron's log line is `did: 1112398686, iid: prop.2.1`, so it lands on
  the read-through side and its predicate goes false with no change to the cron.

- **The rule runner's idempotent precheck**, which is the worse of the two and
  entirely inside this repo. A stale value equal to the target reports success,
  skips the action, and returns *before* any `_write_action_ledger` call. Power
  cut leaves a light's last known value at `on`; a rule that turns it on reads
  the stale `on`, skips, and never retries once power returns. The action is
  lost permanently and leaves no trace. Its existing `code == 0` test needs no
  change: at `ds=2` the cloud returns `-704042011` and the check simply fails.

Cost, measured (15 properties per device, median): online 167–249ms vs 61ms;
offline 67–90ms — not slower, because the cloud rejects from its own registry
instead of waiting for a timeout. Thirty devices concurrently: 290ms at ds=2
against 356ms at ds=1, no rate limiting.

The cloud attaches `updateTime` to every state property and `service.py` was
discarding it. It is now passed through as `updated_at`, verbatim, with `None`
when absent — `siid=1` static metadata has no such field and a 0 would make
callers compute an age of 56 years. The CLI derives `age_s` beside it, because
an agent reading `1769919538` learns nothing. It is documented as an as-of
annotation and explicitly not a liveness signal, since online devices can carry
old timestamps legitimately.

`SKILL.md` narrowed "offline devices: send the command anyway" to the control
path and added the query-side rule. The docs were the proximate cause; leaving
them would undo the rest.

Rejected: synthesizing `-704042011` for offline devices (fabricates a code the
cloud never sent, judged on an unreliable cached flag, and overwrites the real
per-property results the cloud does return), and adding an `online`/`stale` flag
as a gate (same unreliable source; usable as information, not as a verdict).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reading through to a device (`datasource=2`) is all-or-nothing under a ~4.2s
server-side deadline. Ask for more properties than that device can answer
inside the window and the *whole* request comes back as a failure with no
`updateTime` — a healthy, online device reported as unreadable.

Measured on the live account, deterministic and per-device:

    air conditioner   14 props ok  369ms  ->  15 props  -704220043  4374ms
    air purifier      16 props ok  857ms  ->  20 props  -704220043  4739ms
    washer            16 props ok  368ms  ->  20 props  -704220043  4179ms

`GET /devices/{did}/status?iid=` takes a comma-separated list (router.py:316),
and 11 of the 33 devices in this home expose 15+ readable properties. So one
targeted question — "what is the air conditioner set to?" — falls off the
cliff and loses the entire device. That trades a loud false positive for a
silent false negative, which is the exact direction the read-through change
exists to avoid.

`get_props_async` now sends `ds=2` in batches of 8 and concatenates the
results in request order, so callers can still match them up positionally. 8
is half of the lowest cliff measured (15), leaving room for a device having a
bad day; the constant carries that derivation so nobody raises it from a
guess. `ds=1` reads the cloud's cache, has no such cliff (15 properties in
61ms) and stays a single request — splitting it would only slow the panel's
cold refresh and push it toward MiOT's ~10 QPS limit.

Same devices, whole readable set, before and after:

    bedroom camera   71 props  6038ms  0/71  ->  1400ms  71/71
    study camera     43 props  4138ms  0/43  ->   889ms  40/43
    air conditioner  42 props  4198ms  0/42  ->  1758ms  42/42

Batching is *faster* than not batching, because a doomed request burns the
full deadline first. The three failures left on the study camera are
per-property `-704220008` from the cloud — real answers that the whole-batch
failure had been masking. (The 71-property camera failed with `-704083036`
rather than `-704220043`; same cliff, different code.)

Batches go out sequentially. Six concurrent read-throughs to one device add
load to the device and move toward the rate limit for no gain at this size.

A batch that comes back carrying error codes does not stop the remaining
batches, and its codes are passed through verbatim — only codes the cloud
actually returned count as failure (XiaoMi#394), so neither swallowing them nor
rewriting them is allowed. A transport-level exception still propagates:
that means "not read", and a response quietly missing a few rows is exactly
the failure mode this whole change exists to eliminate.

The unit test sits under `backend/miloco/tests/` because
`backend/pyproject.toml` sets `norecursedirs = ["miot", ...]` — anything in
`backend/miot/tests/` is never collected, so a test there would not run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

👋 感谢提交 PR @LeonJoeeee!维护者会尽快 review。

提交前请确认:

  • CI 全绿(test / lint / build)
  • 改动聚焦单一主题,便于审阅
  • 若改动了依赖(lockfile / pyproject.toml / package.json),需维护者评论 /allow-dependencies-change <当前 head SHA> 放行(之后再 push 需重新放行)

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

[PR #485]: fix(miot): 不再把缓存里的「上次已知值」当成设备的当前值端出去

作者: LeonJoeeee
范围: fix/stale-offline-device-reads → main

修改方案

要解决的问题:MIoT 云端对不可达设备返回"最后已知值 + code: 0",响应中没有任何标记可以区分"设备就是这个值"与"读不到设备"。事故:主卧灯离线后缓存永久停在 on=true,定时任务在 00:00~02:30 每 30 分钟朝卧室音箱播报睡眠提醒。

整体方案:沿三条正交主线展开。

  1. 引入 datasource 参数区分"读缓存"与"读真机"

    • 在云端 HTTP 客户端 get_props_async 增加 datasource 参数(默认 1,向后兼容);ds=2 触发设备侧真实读回,离线设备由云端返回 -704042011 而非陈旧值
    • 包装层 get_device_properties 同步增加 datasource 透传
  2. 按调用方意图分档路由 + 幂等预检强制读真机

    • 服务层 get_device_status 复用已有的 user_specified(是否传了 iid)作为分档线:传了 iid 说明调用方在拿这个值当判据,走 ds=2;不传 iid 是面板全量冷查询,走 ds=1 避免撞限频
    • 规则引擎幂等预检 _execute_action 强制 datasource=2:否则断电后缓存停在 on → 读到陈旧 on → 判定"已在目标态" → 跳过写入 → 台账一行都不落
  3. ds=2 的批量悬崖处理 + 新鲜度透出

    • 读真机有约 4.2s 服务端截止时限,超过该设备能答完的属性数整批返回 -704220043。悬崖位置因设备而异(实测:空调 14→15、净化器/洗衣机 16→20)。引入常量 READ_THROUGH_MAX_PROPS_PER_REQ = 8(最低悬崖的一半),在 get_props_asyncds=2 顺序分批、结果按请求顺序拼接;ds=1 不分批
    • 透出云端自带的 updateTime 字段(service.pyupdated_at),缺失给 None 而非 0;CLI 层 device.py 换算成 age_s(多少秒前),方便 agent 消费
    • SKILL.md 收窄"离线设备照常下命令"到控制路径,新增第 5 条规则"读数不等于现状"

测试覆盖

主线 测试文件 用例摘要
批量悬崖 test_miot_read_through_batching.py(新) 超过 chunk 必须全部成功返回、顺序不变、单批失败不影响其它批且错误码原样返回、ds=1 不被拆、不足一批不多花往返、chunk 值不得高于实测最低悬崖
分档路由 test_miot_service_lru.py 定点查询 ds=2、全量冷查询 ds=1updated_at 透传与缺失给 None、云端码不被改写
幂等预检 test_rule.py 幂等预检 ds=2、不可达设备不被误判为"已在目标态"
age_s test_commands.py age_s 换算、缺失时间戳时不补

问题

🔵 建议(可选优化)

  • backend/miot/src/miot/cloud.py:914 — 分批循环中传输异常会丢弃已成功的批次结果

    • 背景get_props_async 的顺序分批循环里,__get_props_once_async 若抛传输层异常(网络断、DNS 失败),循环中断、已成功批次的结果随栈帧丢弃。调用方拿到的是异常而非部分结果。
    • 问题:对 42 属性请求(按 8 分成 6 批),若第 5 批网络抖动,前 32 条已读到的有效值全部丢失。调用方无法区分"完全没读到"与"读到了大部分但有一批网络异常"。
    • 改进:可以考虑用 try/except 捕获单批异常、将该批属性以错误码占位返回,让成功批次结果不丢。不过当前设计选择(传输异常 = "没读到",上抛)与 PR 的原则"悄悄少几行比整体失败危险得多"一致,整体失败至少不会给调用方一个不完整的视图。仅作可选优化。
  • cli/src/miloco_cli/commands/device.py:485age_s 依赖 CLI 本地时钟与云端时钟同步

    • 背景age_s = max(0, now - ts)now 来自 CLI 机器的 time.time()ts 是云端的 updateTime(epoch 秒)。两者在不同机器上。
    • 问题:若 CLI 所在机器时钟比云端快 N 秒,age_s 会偏大 N 秒;若慢 N 秒则偏小。max(0, ...) 防止了负值,但小幅漂移(几秒~几十秒)在判据场景("灯的状态还是 3 天前的")中影响可忽略。
    • 改进:当前设计合理——age_s 是 CLI 侧的便利字段,updated_at 才是权威值。若后续发现时钟漂移显著,可改为后端计算(后端与云端时钟通常更接近)。

结论

LGTM — 方案清晰、动机有事故支撑、实现与描述高度一致、测试覆盖全面。三个分层(分档路由 / 批量悬崖 / 新鲜度透出)正交且各自有独立测试。代码注释是项目里少见的"把 trade-off 写在现场"风格,后续维护友好。


由 review-pr skill v1.6 生成

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GET /api/miot/devices/{did}/status 把「上次已知值」按「当前值」的形状端出来 —— agent 会把它当事实断言给用户

1 participant