Skip to content

Commit a8785ad

Browse files
committed
fix(diag): byte 66 bit 7 is a mode flag, not a fault
Engaging 8 C frost-guard heat made `faults` report FAULT(S) PRESENT on a perfectly healthy unit. Byte 66 bit 7 sets while frost-guard is engaged and clears when it is released, observed on hardware during the t_8heat bring-up. Byte 66 is mapped as fault group 0x33, and HisenseFaults.any deliberately ORs the RAW bytes so that an unnamed bit still counts as a fault. That rule is right, but it made a mode flag read as a fault. Masked with HISENSE_FAULT_NONFAULT_PROTECT, and the console's unnamed-bit reporting aligned to match. The mask is deliberately narrow: ONLY bits proven not to be faults belong in it. Unnamed bits stay counted, because a bit we cannot name is still the A/C reporting something, and answering 'healthy' because we lack a name for it is the worst failure a diagnostics feature can have. This is an exception to that rule, not a relaxation of it, and the tests pin that distinction: bit 7 alone does not raise a fault, bit 4 still does, the two together still do, and any OTHER unnamed bit in the same byte still does. Worth noting how this was caught: the frame-diff tool prints UNNAMED for bits it has no name for rather than ignoring them. A tool that reported only known bits would have hidden it, and the false alarm would have shipped. Tests: 316 codec (was 310). Assisted-by: AI
1 parent 0d18d3a commit a8785ad

5 files changed

Lines changed: 46 additions & 3 deletions

File tree

firmware/esp32-matter/main/diag_console.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,10 @@ static int cmd_faults(int, char **)
267267
};
268268
for (auto &x : named) if (x.v) printf(" FAULT: %s\r\n", x.n);
269269
// A set bit with no name still matters: warn rather than imply we understood the byte.
270-
uint8_t known[4] = { 0xFF, 0xF8, 0x78, 0x10 };
270+
// Byte 66 bit 7 is a mode flag, not a fault, so it must not be reported as an unnamed
271+
// fault bit either (see HISENSE_FAULT_NONFAULT_PROTECT).
272+
uint8_t known[4] = { 0xFF, 0xF8, 0x78,
273+
(uint8_t)(0x10 | HISENSE_FAULT_NONFAULT_PROTECT) };
271274
uint8_t rawb[4] = { f.raw_indoor, f.raw_module, f.raw_outdoor, f.raw_protect };
272275
int off[4] = { HISENSE_FAULT_BYTE_INDOOR, HISENSE_FAULT_BYTE_MODULE,
273276
HISENSE_FAULT_BYTE_OUTDOOR, HISENSE_FAULT_BYTE_PROTECT };

firmware/src/rs485-driver/hisense_rs485.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,12 @@ bool hisense_parse_faults(const uint8_t *buf, size_t len, HisenseFaults *out)
325325
// `any` deliberately ORs the RAW bytes, not the named bits: an undocumented bit in
326326
// one of these bytes is still the A/C reporting something wrong, and reporting
327327
// "healthy" because we lack a name for it would be the worst possible failure.
328-
out->any = (out->raw_indoor | out->raw_module | out->raw_outdoor | out->raw_protect) != 0;
328+
//
329+
// The one exception is byte 66 bit 7, which is a MODE flag (set while 8 C frost-guard
330+
// heat is engaged), proven on hardware. Without masking it, running frost-guard made
331+
// this report a fault on a healthy unit. See HISENSE_FAULT_NONFAULT_PROTECT.
332+
out->any = (out->raw_indoor | out->raw_module | out->raw_outdoor
333+
| (uint8_t)(out->raw_protect & ~HISENSE_FAULT_NONFAULT_PROTECT)) != 0;
329334
out->valid = true;
330335
return true;
331336
}

firmware/src/rs485-driver/hisense_rs485.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,19 @@ static inline bool hisense_setpoint_in_range(int8_t setpoint, bool fahrenheit)
588588
#define HISENSE_FAULT_BYTE_OUTDOOR 64 /* payload 0x31 */
589589
#define HISENSE_FAULT_BYTE_PROTECT 66 /* payload 0x33 */
590590

591+
/* Bits in a fault byte that are NOT faults.
592+
*
593+
* Byte 66 bit 7 sets while 8 C frost-guard heat is ENGAGED and clears when it is released
594+
* (observed on hardware 2026-07-19 during the t_8heat bring-up). It is a mode flag sharing
595+
* the byte, not a fault. Counting it made `faults` report FAULT(S) PRESENT on a perfectly
596+
* healthy unit that happened to be in frost-guard mode.
597+
*
598+
* Only bits PROVEN not to be faults belong here. Unnamed bits stay counted on purpose: a
599+
* bit we cannot name is still the A/C reporting something, and answering "healthy" because
600+
* we lack a name for it is the worst failure a diagnostics feature can have. This mask is
601+
* the narrow exception to that rule, not a relaxation of it. */
602+
#define HISENSE_FAULT_NONFAULT_PROTECT 0x80
603+
591604
typedef struct {
592605
bool valid; // a long-enough status frame was parsed
593606
bool any; // true if ANY fault bit is set (cheap "is it healthy")

firmware/src/version.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
1.2.23
1+
1.2.24

firmware/test/test_codec.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,28 @@ int main() {
491491
CHECK(fl.any, "%s also raises any", c.name);
492492
}
493493

494+
// Byte 66 bit 7 is a MODE flag (8 C frost-guard engaged), not a fault. Observed on
495+
// hardware setting and clearing with the mode. Counting it reported a fault on a
496+
// healthy unit in frost-guard, which is the diagnostics feature crying wolf.
497+
make_status(s, true, HISENSE_MODE_HEAT, 22, 25, 0x01, 0x00, 0x00, 0x00, 40, 30);
498+
s[HISENSE_FAULT_BYTE_PROTECT] = 0x80;
499+
CHECK(hisense_parse_faults(s, 160, &fl), "frost-guard frame parses");
500+
CHECK(!fl.any, "byte66 bit7 alone does NOT report a fault (it is 8C heat engaged)");
501+
CHECK(fl.raw_protect == 0x80, "raw byte still preserved for inspection");
502+
// ...but a REAL fault in the same byte must still register, and so must the
503+
// combination of the mode flag and a real fault.
504+
s[HISENSE_FAULT_BYTE_PROTECT] = 0x10;
505+
CHECK(hisense_parse_faults(s, 160, &fl) && fl.any && fl.over_temp,
506+
"byte66 bit4 still reports over-temp");
507+
s[HISENSE_FAULT_BYTE_PROTECT] = 0x90;
508+
CHECK(hisense_parse_faults(s, 160, &fl) && fl.any && fl.over_temp,
509+
"mode flag + real fault together still reports the fault");
510+
// An unnamed bit in that byte OTHER than bit 7 must still count: the mask is a
511+
// narrow proven exception, not a blanket relaxation.
512+
s[HISENSE_FAULT_BYTE_PROTECT] = 0x01;
513+
CHECK(hisense_parse_faults(s, 160, &fl) && fl.any,
514+
"other unnamed bits in byte66 still raise any");
515+
494516
// An UNNAMED bit must still raise `any`. Reporting "healthy" because we have no
495517
// name for a bit would be the worst possible failure of a diagnostics feature.
496518
make_status(s, true, HISENSE_MODE_COOL, 22, 25, 0x01, 0x00, 0x00, 0x00, 40, 30);

0 commit comments

Comments
 (0)