Skip to content

Commit 3bb2134

Browse files
authored
Merge pull request #68 from AndrewDemsDS/fix/breakglass-and-subscription-revert
Release: AmebaZ2 v1.3.3 + ESP32 v1.1.0 — C/F write path, contact-sensor polarity, and a "77" recommission that never worked
2 parents 7f71c4c + 2756141 commit 3bb2134

11 files changed

Lines changed: 798 additions & 89 deletions

File tree

firmware/esp32-matter/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# . ~/esp/esp-idf-v5.5.4/export.sh && . $ESP_MATTER_PATH/export.sh
33
# idf.py set-target esp32 && idf.py build flash monitor
44
cmake_minimum_required(VERSION 3.16)
5-
set(PROJECT_VER "1.0.23")
5+
set(PROJECT_VER "1.1.0")
66

77
# Unified versioning (issue #77): the Matter softwareVersion INT is DERIVED from PROJECT_VER --
88
# MAJOR*10000+MINOR*100+PATCH -> a readable, strictly-monotonic uint32. This keeps the human

firmware/esp32-matter/main/app_main.cpp

Lines changed: 303 additions & 38 deletions
Large diffs are not rendered by default.

firmware/esp32-matter/main/diag_console.cpp

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,17 @@ static void print_state(FILE *out, const HisenseState *st)
4848
fprintf(out,
4949
" power=%d mode=%s set=%dC in=%dC out=%dC coil=%dC fan=0x%02x comp=%dHz\r\n"
5050
" eco=%d turbo=%d mute=%d sleep=%d(0x%02x) vswing=%d hswing=%d heatrelay=%d I=%u V=%u\r\n"
51-
" unit=%s (#5, byte26 bit1; UNVERIFIED -- flip the remote to F and diff `raw`)\r\n",
51+
" unit=%s (#5, byte26 bit1; UNVERIFIED -- flip the remote to F and diff `raw`)\r\n"
52+
" link_req=0x%02x (\"77\" bits in the 0x1E reply: 0x08 reconfig / 0x20 smartcfg;\r\n"
53+
" 0x00 = the A/C is NOT asking to recommission -- if it stays 0 while you press the\r\n"
54+
" remote sequence, the request never reaches us and the fault is upstream of Matter)\r\n",
5255
st->power_on, mode_name(st->mode), st->setpoint_c, st->indoor_temp_c,
5356
st->outdoor_temp_c, st->coil_temp_c, st->fan_raw, st->compressor_freq,
5457
st->eco_on, st->turbo_on, st->mute_on, st->sleep_on, st->sleep_raw,
5558
st->vswing_on, st->hswing_on, st->heat_relay_on,
5659
(unsigned)st->current_raw, (unsigned)st->voltage_raw,
57-
st->temp_unit_f ? "F" : "C");
60+
st->temp_unit_f ? "F" : "C",
61+
(unsigned) hisense_get_last_link_req());
5862
}
5963

6064
extern "C" void diag_on_status(const HisenseState *st)
@@ -284,6 +288,30 @@ static int cmd_faults(int, char **)
284288
// Hexdump the last status frame. The point of this is falsifiability: the fault map is
285289
// derived from firmware, so being able to read the actual bytes is what lets someone
286290
// prove it wrong. Also the general tool for mapping any still-unknown status field.
291+
/* Hexdump the last 0x1E LINK reply. This is the frame that is SUPPOSED to carry the "77"
292+
* recommission request in payload[4] (byte 17). On this unit seven remote presses never moved
293+
* that byte, so dump the whole frame, press the sequence, dump again, and diff: whichever byte
294+
* actually changes is the real request. Beats trusting an RE bit map that has already been
295+
* wrong elsewhere in this protocol. */
296+
static int cmd_link(int, char **)
297+
{
298+
uint8_t f[40];
299+
uint8_t n = hisense_get_last_link_frame(f, sizeof(f));
300+
if (!n) { printf("no 0x1E LINK reply captured yet\r\n"); return 0; }
301+
printf("last 0x1E LINK reply, %u bytes:\r\n", (unsigned) n);
302+
// int counters on purpose: `k < i + 16` promotes the RHS to int, so a uint8_t k could not
303+
// reach it once i+16 passed 255 and the loop would never terminate (CodeQL, PR #68).
304+
for (int i = 0; i < (int) n; i += 16) {
305+
printf(" %3d:", i);
306+
for (int k = i; k < i + 16 && k < (int) n; k++) printf(" %02x", f[k]);
307+
printf("\r\n");
308+
}
309+
printf(" payload[4] = byte[17] = 0x%02x (documented \"77\" bits: 0x08 reconfig / 0x20 smartcfg)\r\n",
310+
n > 17 ? f[17] : 0);
311+
printf(" masked link_req = 0x%02x\r\n", (unsigned) hisense_get_last_link_req());
312+
return 0;
313+
}
314+
287315
static int cmd_raw(int, char **)
288316
{
289317
uint8_t f[HISENSE_RAW_SNAPSHOT_LEN];
@@ -442,6 +470,7 @@ extern "C" void diag_console_start(void)
442470
{ "bootreason", "#12: why the module last rebooted (brownout / panic / OTA)",
443471
NULL, cmd_bootreason, NULL, NULL },
444472
{ "raw", "hexdump the last status frame (falsifies the fault map)", NULL, cmd_raw, NULL, NULL },
473+
{ "link", "hexdump the last 0x1E LINK reply (find the real \"77\" bit)", NULL, cmd_link, NULL, NULL },
445474
{ "tx", "#52 bench probe: tx <offset> <value> — current frame, one byte overridden",
446475
NULL, cmd_tx, NULL, NULL },
447476
};

firmware/esp32-matter/sdkconfig.defaults

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ CONFIG_CUSTOM_DEVICE_INFO_PROVIDER=y
4646
# so it (and the fallback NUMBER) MUST stay equal to PROJECT_VER or the device reports a stale
4747
# softwareVersionString. Edit PROJECT_VER in CMakeLists.txt, then update both lines below.
4848
# esp32-lint.sh enforces this equality (fails the commit/CI if they drift).
49-
CONFIG_DEVICE_SOFTWARE_VERSION_NUMBER=10023
50-
CONFIG_DEVICE_SOFTWARE_VERSION_STRING="1.0.23"
49+
CONFIG_DEVICE_SOFTWARE_VERSION_NUMBER=10100
50+
CONFIG_DEVICE_SOFTWARE_VERSION_STRING="1.1.0"
5151
# --- OTA hardening (faster + reliable on marginal Wi-Fi) ---
5252
# Delta OTA: ship a diff (tens of KB) instead of the full ~1.5MB image over BDX.
5353
CONFIG_ENABLE_DELTA_OTA=y

firmware/scripts/esp32-release.sh

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,61 @@ build() {
121121
say "#82: no prior release recorded -- first build, nothing to preserve"
122122
fi
123123

124+
# Flavour. node 28 runs the DEBUG flavour on purpose (it is the dev unit), and the :2323 diag
125+
# console + `tx` bench probe are gated on CONFIG_HISENSE_DEBUG_BUILD, which lives ONLY in
126+
# sdkconfig.debug. Building without that overlay silently ships an image with no console -- and
127+
# on a node whose Matter link is flaky, that console is the only way in. Default to debug here
128+
# for exactly that reason. Opt out with ESP32_FLAVOUR=release (an env var, NOT a --release
129+
# flag -- this script does not parse one).
130+
# Recovery credentials. The Identify=88 OTA fetch target and the :2324 break-glass listener are
131+
# both baked at BUILD time and are the only two remote ways back into this node. Building without
132+
# them silently ships an image whose listener never opens and whose OTA URL is a non-resolvable
133+
# placeholder: the device looks healthy and is quietly USB-only.
134+
#
135+
# Not hypothetical -- every esp32 build on 2026-07-20 used bare `idf.py build`, so node 35 ended
136+
# up with :2324 closed and a placeholder URL and could not be updated over the air at all.
137+
#
138+
# NAME NORMALISATION MATTERS HERE. ota-release.env defines BREAKGLASS_TOKEN / BREAKGLASS_PORT
139+
# (what ota-release.sh reads for the ameba half), but esp32-matter/CMakeLists.txt consumes
140+
# HISENSE_BREAKGLASS_TOKEN / HISENSE_BREAKGLASS_PORT. Checking one name and exporting neither is
141+
# a guard that PASSES while still building a listener-less image -- the very failure it exists to
142+
# catch. So: accept either spelling, then export the HISENSE_* names the build actually reads.
143+
: "${HISENSE_BREAKGLASS_TOKEN:=${BREAKGLASS_TOKEN:-}}"
144+
: "${HISENSE_BREAKGLASS_PORT:=${BREAKGLASS_PORT:-}}"
145+
export HISENSE_OTA_URL HISENSE_BREAKGLASS_TOKEN HISENSE_BREAKGLASS_PORT
146+
147+
if [ "${ESP32_ALLOW_NO_RECOVERY:-0}" != "1" ]; then
148+
[ -n "${HISENSE_OTA_URL:-}" ] \
149+
|| die "HISENSE_OTA_URL is unset -- the Identify=88 OTA fetch would bake a placeholder URL and
150+
the image would be USB-only. Set it (ota-release.env or the environment), or pass
151+
ESP32_ALLOW_NO_RECOVERY=1 if you really want a bench image with no remote recovery."
152+
[ -n "${HISENSE_BREAKGLASS_TOKEN:-}" ] \
153+
|| die "no break-glass token (set BREAKGLASS_TOKEN or HISENSE_BREAKGLASS_TOKEN) -- the :2324
154+
listener fails closed and never opens, so the image would be USB-only. Set it, or pass
155+
ESP32_ALLOW_NO_RECOVERY=1."
156+
say "recovery credentials present (OTA URL + break-glass token exported to the build)"
157+
else
158+
say "WARNING: ESP32_ALLOW_NO_RECOVERY=1 -- image will have NO remote recovery path (USB only)"
159+
fi
160+
161+
local sdkdef="sdkconfig.defaults"
162+
if [ "${ESP32_FLAVOUR:-debug}" = "debug" ]; then
163+
sdkdef="sdkconfig.defaults;sdkconfig.debug"
164+
say "flavour: DEBUG (:2323 console + tx probe)"
165+
else
166+
say "flavour: RELEASE (no console) -- node 28 normally wants debug"
167+
fi
168+
124169
say "idf.py build ($semver, int $int)"
125-
( cd "$ESP" && idf.py set-target esp32 && idf.py build )
170+
( cd "$ESP" && idf.py -DSDKCONFIG_DEFAULTS="$sdkdef" set-target esp32 \
171+
&& idf.py -DSDKCONFIG_DEFAULTS="$sdkdef" build )
126172
[ -f "$NEW_BIN" ] || die "build produced no $NEW_BIN"
173+
# Fail loudly rather than shipping a consoleless image by accident.
174+
if [ "${ESP32_FLAVOUR:-debug}" = "debug" ]; then
175+
grep -q '^CONFIG_HISENSE_DEBUG_BUILD=y' "$ESP/sdkconfig" \
176+
|| die "debug flavour requested but CONFIG_HISENSE_DEBUG_BUILD is not set in the generated sdkconfig -- the :2323 console would be MISSING from this image"
177+
say "verified: CONFIG_HISENSE_DEBUG_BUILD=y (console present)"
178+
fi
127179

128180
local archive="$IMG/esp32-hisense_ac_matter-v$semver.bin"
129181
mkdir -p "$IMG"; cp "$NEW_BIN" "$archive"

firmware/src/rs485-driver/hisense_rs485.cpp

Lines changed: 77 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ static hisense_status_cb_t s_status_cb = NULL;
3434
static TaskHandle_t s_bus_task_handle = NULL;
3535
static bool s_initialized = false;
3636
static hisense_recommission_cb_t s_recommission_cb = NULL;
37+
static hisense_recommission_cancel_cb_t s_recommission_cancel_cb = NULL;
38+
/* Whole last 0x1E LINK reply, so a bench can diff the frame across a "77" press instead of
39+
* trusting the RE bit map. Seven presses left the documented bit (payload[4] 0x08/0x20) at
40+
* zero, so the map is unproven on THIS unit and the raw frame is the only ground truth. */
41+
static uint8_t s_last_link_frame[40];
42+
static uint8_t s_last_link_frame_len = 0;
43+
static hisense_link_frame_cb_t s_link_frame_cb = NULL;
44+
static bool s_recommission_momentary = false; // latched by a 1-frame pulse
3745
static uint8_t s_last_link_req = 0; // masked "77" request bits from the last 0x1E reply (diag)
3846
static uint8_t s_link_req_streak = 0; // consecutive 0x1E replies with the request asserted (debounce)
3947
static bool s_recommission_latched = false; // fired for the current sustained assertion (fire-once)
@@ -1047,22 +1055,28 @@ static void hisense_consume_status(size_t n)
10471055
// so a single reflected/glitched frame must not trip a commissioning window. The
10481056
// request stays asserted while the user holds "77", so a few frames of hold cleanly
10491057
// separates a deliberate press from a 1-frame artefact. Not static: linked by tests.
1050-
bool hisense_recommission_debounce(uint8_t req_bits, uint8_t *streak,
1051-
bool *latched, uint8_t hold_frames)
1058+
int hisense_recommission_debounce(uint8_t req_bits, uint8_t *streak,
1059+
bool *latched, uint8_t hold_frames)
10521060
{
10531061
if (req_bits != 0) {
10541062
if (*streak < 0xFF) {
10551063
(*streak)++;
10561064
}
10571065
if (*streak >= hold_frames && !*latched) {
10581066
*latched = true;
1059-
return true; // fire once for this sustained assertion
1067+
return 1; // fire once for this sustained assertion
10601068
}
1061-
return false;
1069+
return 0;
10621070
}
1063-
*streak = 0; // request cleared -> reset + re-arm
1071+
/* Request cleared. If we had LATCHED, the user has taken the A/C out of "77" -- report the
1072+
* falling edge so the handler can shut the commissioning window it opened. Without this the
1073+
* window stayed open for its full duration after the user backed out, leaving the device
1074+
* joinable with nothing on the panel to indicate it. A clear that arrives BEFORE the latch
1075+
* (a glitch that never fired) reports nothing, because no window was ever opened. */
1076+
bool was_latched = *latched;
1077+
*streak = 0; // reset + re-arm
10641078
*latched = false;
1065-
return false;
1079+
return was_latched ? -1 : 0;
10661080
}
10671081

10681082
// Link-health edge detector (pure -> host-testable, #56). `silent` = link currently
@@ -1092,21 +1106,74 @@ static void hisense_check_link_reply(size_t n)
10921106
if (n <= 17 || s_msg_buf[13] != 0x1E) {
10931107
return;
10941108
}
1109+
{ // snapshot the whole reply for bench diffing (see s_last_link_frame)
1110+
size_t cp = (n < sizeof(s_last_link_frame)) ? n : sizeof(s_last_link_frame);
1111+
for (size_t i = 0; i < cp; i++) s_last_link_frame[i] = s_msg_buf[i];
1112+
s_last_link_frame_len = (uint8_t) cp;
1113+
if (s_link_frame_cb != NULL) s_link_frame_cb(s_last_link_frame, s_last_link_frame_len);
1114+
}
10951115
uint8_t req = s_msg_buf[17] & HISENSE_LINK_REQ_RECOMMISSION; // payload[4]
10961116
// Re-arm if the request TYPE changes while still asserted (e.g. RECONFIG->SMARTCFG
10971117
// with no intervening all-zero frame) so the second reason isn't swallowed by the latch.
10981118
if (req != 0 && req != s_last_link_req) {
10991119
s_link_req_streak = 0;
11001120
s_recommission_latched = false;
11011121
}
1102-
if (hisense_recommission_debounce(req, &s_link_req_streak, &s_recommission_latched,
1103-
HISENSE_RECOMMISSION_HOLD_FRAMES)
1104-
&& s_recommission_cb != NULL) {
1105-
s_recommission_cb(s_msg_buf[17]);
1122+
/* Hold requirement depends on WHICH bit asserted, because only one of them can echo.
1123+
*
1124+
* bit3 (0x08, reconfig) doubles as our OUTBOUND prov_status, so a reflected or glitched
1125+
* frame could trip a commissioning window -- it needs the multi-frame hold.
1126+
*
1127+
* bit5 (0x20, smartcfg) is never transmitted by us: it is purely an A/C-originated request,
1128+
* so there is nothing to echo and nothing to debounce. Measured on the bench 2026-07-20:
1129+
* this unit asserts 0x20 for EXACTLY ONE ~1Hz frame per remote press (three presses ->
1130+
* LINK#93, #100, #114, never consecutive). Requiring 3 consecutive frames therefore made
1131+
* "77" impossible to trigger here -- the handler never once fired. Fire on the first frame.
1132+
*
1133+
* Mixed bits keep the conservative hold: if 0x08 is present the echo risk is present too. */
1134+
uint8_t hold = (req == HISENSE_LINK_REQ_SMARTCFG) ? 1 : HISENSE_RECOMMISSION_HOLD_FRAMES;
1135+
int edge = hisense_recommission_debounce(req, &s_link_req_streak, &s_recommission_latched,
1136+
hold);
1137+
if (edge > 0) {
1138+
/* Remember whether this was a MOMENTARY request. The A/C pulses 0x20 for one frame and
1139+
* drops it, so the very next reply is a falling edge -- which must NOT be read as "the
1140+
* user left 77", or the window would slam shut ~1s after opening. A sustained request
1141+
* (0x08 held for the hold period) genuinely does track the user's state, so its falling
1142+
* edge IS meaningful. */
1143+
s_recommission_momentary = (hold == 1);
1144+
if (s_recommission_cb != NULL) s_recommission_cb(s_msg_buf[17]);
1145+
} else if (edge < 0) {
1146+
bool momentary = s_recommission_momentary;
1147+
s_recommission_momentary = false;
1148+
if (!momentary && s_recommission_cancel_cb != NULL) {
1149+
s_recommission_cancel_cb(); // sustained request released -> user left "77"
1150+
}
11061151
}
11071152
s_last_link_req = req; // retained for diagnostics + reason-change detection
11081153
}
11091154

1155+
void hisense_set_link_frame_cb(hisense_link_frame_cb_t cb)
1156+
{
1157+
s_link_frame_cb = cb;
1158+
}
1159+
1160+
uint8_t hisense_get_last_link_frame(uint8_t *out, uint8_t cap)
1161+
{
1162+
uint8_t n = s_last_link_frame_len < cap ? s_last_link_frame_len : cap;
1163+
for (uint8_t i = 0; i < n; i++) out[i] = s_last_link_frame[i];
1164+
return n;
1165+
}
1166+
1167+
uint8_t hisense_get_last_link_req(void)
1168+
{
1169+
return s_last_link_req;
1170+
}
1171+
1172+
void hisense_set_recommission_cancel_cb(hisense_recommission_cancel_cb_t cb)
1173+
{
1174+
s_recommission_cancel_cb = cb;
1175+
}
1176+
11101177
void hisense_set_recommission_cb(hisense_recommission_cb_t cb)
11111178
{
11121179
s_recommission_cb = cb;

firmware/src/rs485-driver/hisense_rs485.h

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,10 +403,19 @@ typedef void (*hisense_status_cb_t)(const HisenseState *state);
403403
#define HISENSE_RECOMMISSION_HOLD_FRAMES 3 // ~3s at ~1Hz: a held "77" press, not a glitch/echo
404404
typedef void (*hisense_recommission_cb_t)(uint8_t reason);
405405

406+
/* Fires when a previously-reported "77" request is DE-asserted, i.e. the user took the A/C back
407+
* out of recommission mode from the panel/remote. The handler should shut whatever commissioning
408+
* window it opened: without this the window stays open for its full duration after the user has
409+
* backed out, leaving the device joinable with nothing on the panel to show it. Only fires if the
410+
* assertion actually fired first -- a glitch that never passed the debounce reports nothing. */
411+
typedef void (*hisense_recommission_cancel_cb_t)(void);
412+
406413
// Pure debounce step for the "77" request (exposed for host tests). Advances the
407414
// streak/latch state for one 0x1E reply's masked request bits and returns true
408415
// exactly once, when the request has been asserted for >= hold_frames in a row.
409-
bool hisense_recommission_debounce(uint8_t req_bits, uint8_t *streak,
416+
// Returns +1 exactly once per sustained assertion, -1 on the falling edge of an assertion that
417+
// had fired (user left "77"), 0 otherwise. Mirrors hisense_link_health_edge's edge convention.
418+
int hisense_recommission_debounce(uint8_t req_bits, uint8_t *streak,
410419
bool *latched, uint8_t hold_frames);
411420

412421
// Pure reply-class gate (exposed for host tests). A transaction accepts a parsed
@@ -463,6 +472,27 @@ int hisense_init(hisense_status_cb_t cb);
463472
// Safe to call before or after hisense_init.
464473
void hisense_set_recommission_cb(hisense_recommission_cb_t cb);
465474

475+
// Optional: notified when the A/C drops the "77" request (see the typedef above).
476+
void hisense_set_recommission_cancel_cb(hisense_recommission_cancel_cb_t cb);
477+
478+
/* Last masked "77" request bits seen in a 0x1E LINK reply (payload[4] & RECOMMISSION mask).
479+
* Diagnostic: without this there is no way to tell "the A/C never asked" from "we missed it",
480+
* which is exactly the ambiguity that made a non-functioning 77 impossible to debug. */
481+
uint8_t hisense_get_last_link_req(void);
482+
483+
/* Copies the whole last 0x1E LINK reply into `out` (up to `cap`), returns the length. The
484+
* documented "77" bit map is UNPROVEN on this unit -- seven remote presses never moved
485+
* payload[4] -- so dump the frame and diff it across a press to find the byte that really
486+
* changes, rather than trusting the RE map. */
487+
uint8_t hisense_get_last_link_frame(uint8_t *out, uint8_t cap);
488+
489+
/* Fires on EVERY 0x1E LINK reply, with the raw frame. These replies are infrequent and
490+
* irregular, so a polled snapshot cannot catch a change caused by a button press -- by the time
491+
* you poll, either nothing new arrived or the interesting frame is long gone. A push callback
492+
* lets a bench log every reply continuously and diff them offline. */
493+
typedef void (*hisense_link_frame_cb_t)(const uint8_t *frame, uint8_t len);
494+
void hisense_set_link_frame_cb(hisense_link_frame_cb_t cb);
495+
466496
// Report provisioning state to the A/C: while true, the outbound 0x1E carries
467497
// prov_status=1 (payload[4] bit3), which lights "77" on the A/C panel (the module
468498
// telling the mainboard it's pairing). Set true when the commissioning window opens,

0 commit comments

Comments
 (0)