From 8433f46d569694ab136fc90696031ec68ef4270d Mon Sep 17 00:00:00 2001 From: Paul Swenson <1937248+pdscomp@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:12:20 -0400 Subject: [PATCH 01/11] hx711s: make frame validity explicit Transmit raw channel counts with MCU-owned frame quality metadata, use deterministic reset and settling qualification, and abort an active load-cell probe immediately on invalid acquisition. Remove host-side held-value synthesis and the divergent fixed 100000-count classifier so invalid frames cannot reach tare, filters, or trigger decisions. Also use a 30-sample median tare baseline, require two consecutive qualified filtered samples to trigger, repair command-time SOS filter updates, and harden fixed-point bounds. --- klippy/extras/load_cell/ads1220.py | 1 + klippy/extras/load_cell/ads131m0x.py | 1 + klippy/extras/load_cell/hx711s.py | 110 ++----- klippy/extras/load_cell/hx71x.py | 1 + klippy/extras/load_cell/interfaces.py | 9 + klippy/extras/load_cell/load_cell.py | 18 +- klippy/extras/load_cell/load_cell_probe.py | 21 +- klippy/extras/load_cell/sos_filter.py | 4 +- src/load_cell_probe.c | 27 +- src/load_cell_probe.h | 1 + src/sensor_hx711s.c | 334 ++++++++++----------- src/sos_filter.c | 28 +- 12 files changed, 273 insertions(+), 282 deletions(-) diff --git a/klippy/extras/load_cell/ads1220.py b/klippy/extras/load_cell/ads1220.py index 75a528a3af..51b5316cb6 100644 --- a/klippy/extras/load_cell/ads1220.py +++ b/klippy/extras/load_cell/ads1220.py @@ -240,6 +240,7 @@ def _process_batch(self, eventtime) -> BulkAdcData: "data": samples, "errors": self.last_error_count, "overflows": self.ffreader.get_last_overflows(), + "faults": [], } def reset_chip(self): diff --git a/klippy/extras/load_cell/ads131m0x.py b/klippy/extras/load_cell/ads131m0x.py index 914540d820..4b8ba47f16 100644 --- a/klippy/extras/load_cell/ads131m0x.py +++ b/klippy/extras/load_cell/ads131m0x.py @@ -261,6 +261,7 @@ def _process_batch(self, eventtime): "data": self._convert_samples(samples), "errors": self.last_error_count, "overflows": self.ffreader.get_last_overflows(), + "faults": [], } # --- SPI Communication Helpers --- diff --git a/klippy/extras/load_cell/hx711s.py b/klippy/extras/load_cell/hx711s.py index 2f5f494cc9..95b262fd10 100644 --- a/klippy/extras/load_cell/hx711s.py +++ b/klippy/extras/load_cell/hx711s.py @@ -10,17 +10,10 @@ from klippy.mcu import MCU from .. import bulk_sensor -from .interfaces import BulkAdcData, BulkAdcDataCallback, LoadCellSensor +from .interfaces import AdcFault, BulkAdcData, BulkAdcDataCallback, LoadCellSensor UPDATE_INTERVAL = 0.10 -SAMPLE_ERROR_DESYNC = -0x80000000 -SAMPLE_ERROR_READ_TOO_LONG = 0x40000000 -SAMPLE_ERROR_TORN_READ = 0x20000000 ADC_FACTOR = 1.0 / (1 << 23) -# A corrupt chip frame can pass framing yet sit hundreds of thousands of -# counts from the truth. A single chip's reading moves by well under this -# between samples during a probe, so a larger one-sample jump is a glitch. -CHANNEL_SPIKE_THRESHOLD = 100000 class HX711SBase(LoadCellSensor): @@ -36,13 +29,7 @@ def __init__( self.printer = config.get_printer() self.name = config.get_name().split()[-1] self.sensor_type = sensor_type - self.last_error_count = 0 self.consecutive_fails = 0 - self.torn_read_count = 0 - self.spike_count = 0 - # last valid per-channel counts, held across torn reads and glitches - # to keep the sample stream gap-free (see _convert_samples) - self._last_channel_counts = None ppins = self.printer.lookup_object("pins") sdo_pin_names = [p.strip() for p in config.get("sdo_pins").split(",")] @@ -82,7 +69,9 @@ def __init__( # Bulk sensor setup chip_smooth = self.sps * UPDATE_INTERVAL * 2 - unpack_format = "<" + ("i" * self.sensor_count) + # The final word is MCU-owned frame quality. Raw counts stay in the + # stream for diagnosis, but only quality==0 reaches control clients. + unpack_format = "<" + ("i" * self.sensor_count) + "I" self.ffreader = bulk_sensor.FixedFreqReader(mcu, chip_smooth, unpack_format) self.batch_bulk = bulk_sensor.BatchBulkHelper( @@ -142,59 +131,19 @@ def add_client(self, callback: BulkAdcDataCallback): def attach_load_cell_probe(self, load_cell_probe_oid: int): self.attach_probe_cmd.send([self.oid, load_cell_probe_oid]) - # True if any channel jumped implausibly far from the last good sample. - # Logs which channel(s) glitched so the offending chip can be identified. - def _is_glitch(self, channel_counts, ptime): - last = self._last_channel_counts - if last is None: - return False - bad = [ - i - for i, (c, p) in enumerate(zip(channel_counts, last)) - if abs(c - p) > CHANNEL_SPIKE_THRESHOLD - ] - if not bad: - return False - self.spike_count += 1 - logging.warning( - "%s: glitch dropped at t=%.3f; channel(s) %s jumped" - " (now=%s last=%s)", - self.name, ptime, bad, list(channel_counts), list(last), - ) - return True - def _convert_samples(self, samples): count = 0 + faults: list[AdcFault] = [] for sample in samples: ptime = sample[0] - channel_counts = sample[1:] - val = channel_counts[0] - if val == SAMPLE_ERROR_TORN_READ: - # A chip latched new data mid-read; the MCU flagged it. Hold - # the last valid reading in its place rather than dropping the - # sample, so the stream stays gap-free and uniformly spaced - # for downstream consumers (tap analysis decomposes the force - # curve by index and is fragile to missing points). The held - # value is within one sample period of the true force, so it - # cannot cause a false probe trigger. - self.torn_read_count += 1 - if self._last_channel_counts is None: - continue # nothing to hold yet; drop the leading torn read - channel_counts = self._last_channel_counts - elif val == SAMPLE_ERROR_DESYNC: - self.last_error_count += 1 - logging.error("%s: DESYNC at t=%.3f", self.name, ptime) - break # errors latch in the MCU, the rest are duplicates - elif val == SAMPLE_ERROR_READ_TOO_LONG: - self.last_error_count += 1 - logging.error("%s: READ_TOO_LONG at t=%.3f", self.name, ptime) - break # errors latch in the MCU, the rest are duplicates - elif self._is_glitch(channel_counts, ptime): - # corrupt frame that passed framing; hold the last good - # reading so the curve stays clean for tap analysis - channel_counts = self._last_channel_counts - else: - self._last_channel_counts = channel_counts + channel_counts = tuple(sample[1 : 1 + self.sensor_count]) + quality = sample[1 + self.sensor_count] + if quality: + faults.append( + {"time": round(ptime, 6), "counts": channel_counts, + "quality": quality} + ) + continue converted = [round(ptime, 6)] for ch in channel_counts: converted.append(ch) @@ -202,12 +151,10 @@ def _convert_samples(self, samples): samples[count] = tuple(converted) count += 1 del samples[count:] + return faults def _start_measurements(self): self.consecutive_fails = 0 - self.last_error_count = 0 - # discard any held value from a prior (now power-cycled) stream - self._last_channel_counts = None rest_ticks = self.mcu.seconds_to_clock( 1.0 / (10.0 * self.get_samples_per_second()) ) @@ -228,33 +175,26 @@ def _finish_measurements(self): def _process_batch(self, eventtime) -> BulkAdcData: prev_overflows = self.ffreader.get_last_overflows() - prev_error_count = self.last_error_count samples = self.ffreader.pull_samples() - self._convert_samples(samples) + faults = self._convert_samples(samples) overflows = self.ffreader.get_last_overflows() - prev_overflows - errors = self.last_error_count - prev_error_count - if errors > 0: - # a read error desyncs the chips and may corrupt their gain - # setting; only a power cycle restores a known state - logging.error( - "%s: forced sensor restart due to read error", self.name + errors = len(faults) + if faults: + logging.warning( + "%s: dropped %d invalid HX711 frame(s); latest quality=0x%x", + self.name, errors, faults[-1]["quality"], ) - self._finish_measurements() - self._start_measurements() - elif overflows > 0: + if overflows > 0: self.consecutive_fails += 1 if self.consecutive_fails > 4: - logging.error( - "%s: forced sensor restart due to overflows", self.name - ) - self._finish_measurements() - self._start_measurements() + logging.error("%s: repeated bulk overflows", self.name) else: self.consecutive_fails = 0 return { "data": samples, - "errors": self.last_error_count, - "overflows": self.ffreader.get_last_overflows(), + "errors": errors, + "overflows": overflows, + "faults": faults, } diff --git a/klippy/extras/load_cell/hx71x.py b/klippy/extras/load_cell/hx71x.py index b8b431263c..3ac94f7cc7 100644 --- a/klippy/extras/load_cell/hx71x.py +++ b/klippy/extras/load_cell/hx71x.py @@ -178,6 +178,7 @@ def _process_batch(self, eventtime) -> BulkAdcData: "data": samples, "errors": self.last_error_count, "overflows": self.ffreader.get_last_overflows(), + "faults": [], } diff --git a/klippy/extras/load_cell/interfaces.py b/klippy/extras/load_cell/interfaces.py index 6ab0446fc1..cc256527fc 100644 --- a/klippy/extras/load_cell/interfaces.py +++ b/klippy/extras/load_cell/interfaces.py @@ -11,12 +11,21 @@ from klippy.mcu import MCU +class AdcFault(TypedDict): + """An invalid raw ADC frame kept for diagnostics, never for control.""" + + time: float + counts: tuple[int, ...] + quality: int + + class BulkAdcData(TypedDict): """Dictionary returned by sensors containing raw sensor data""" data: list[tuple[float, ...]] errors: int overflows: int + faults: list[AdcFault] """Clients of the ADC receive a BulkAdcData dictionary and return True or diff --git a/klippy/extras/load_cell/load_cell.py b/klippy/extras/load_cell/load_cell.py index f27ba00a1a..b815ddf612 100644 --- a/klippy/extras/load_cell/load_cell.py +++ b/klippy/extras/load_cell/load_cell.py @@ -687,7 +687,10 @@ def _sensor_data_event(self, msg): ) sample.append(counts) samples.append(sample) - msg = {"data": samples, "errors": errors, "overflows": overflows} + msg = { + "data": samples, "errors": errors, "overflows": overflows, + "faults": [], + } self.clients.send(msg) return True @@ -798,7 +801,18 @@ def avg_counts(self, num_samples: int | None = None) -> tuple[int, ...]: collector.start_collecting(min_time=toolhead.get_last_move_time()) samples, errors = collector.collect_min(num_samples) self.validate_samples(samples, errors) - return self._avg_counts_from_samples(samples) + # A median is resilient to a single physically plausible but bad + # frame. Acquisition faults have already made validate_samples fail. + first_channel_col = SampleStructure.channel_counts_col(0) + channels = [ + sorted(sample[first_channel_col + (2 * channel)] for sample in samples) + for channel in range(self.channel_count) + ] + midpoint = len(samples) // 2 + if len(samples) % 2: + return tuple(values[midpoint] for values in channels) + return tuple((values[midpoint - 1] + values[midpoint]) // 2 + for values in channels) def validate_samples(self, samples, errors): if errors: diff --git a/klippy/extras/load_cell/load_cell_probe.py b/klippy/extras/load_cell/load_cell_probe.py index 9b1590be38..7dc553c9a0 100644 --- a/klippy/extras/load_cell/load_cell_probe.py +++ b/klippy/extras/load_cell/load_cell_probe.py @@ -370,8 +370,9 @@ def update_from_command(self, gcmd): return # update MCU filter from GCode command self._sos_filter.change_filter( - self._active_design.design_filter(gcmd.error) + gcmd_filter.design_filter(gcmd.error) ) + self._active_design = gcmd_filter def get_sos_filter(self) -> sos_filter.SosFilter: return self._sos_filter @@ -391,10 +392,15 @@ def __init__( self._load_cell: LoadCell = load_cell_inst self._sensor = load_cell_inst.get_sensor() self._rest_time = 1.0 / float(self._sensor.get_samples_per_second()) - # Collect 5 x 50hz power cycles of data to average across power noise + # Keep tare_time for existing configurations, but use a 30-sample + # health window by default. At 80 SPS this matches the 375ms + # qualification window used by Elegoo before it arms each tap. self._tare_time_param = floatParamHelper( config, "tare_time", default=5.0 / 50.0, minval=0.01, maxval=1.0 ) + self._tare_samples_param = intParamHelper( + config, "tare_samples", default=30, minval=2, maxval=200 + ) # triggering options self._trigger_force_param = intParamHelper( config, "trigger_force", default=75, minval=10, maxval=250 @@ -422,9 +428,13 @@ def __init__( ) def get_tare_samples(self, gcmd=None) -> int: + # A command may still request a longer time-based window; never let + # legacy tare_time reduce the health qualification below 30 samples. tare_time = self._tare_time_param.get(gcmd) - sps = self._sensor.get_samples_per_second() - return max(2, math.ceil(tare_time * sps)) + time_samples = math.ceil( + tare_time * self._sensor.get_samples_per_second() + ) + return max(self._tare_samples_param.get(gcmd), time_samples) def get_trigger_force_grams(self, gcmd=None) -> int: return self._trigger_force_param.get(gcmd) @@ -525,6 +535,7 @@ class McuLoadCellProbe: ERROR_SAFETY_RANGE = mcu.MCU_trsync.REASON_COMMS_TIMEOUT + 1 ERROR_OVERFLOW = mcu.MCU_trsync.REASON_COMMS_TIMEOUT + 2 ERROR_WATCHDOG = mcu.MCU_trsync.REASON_COMMS_TIMEOUT + 3 + ERROR_ADC_INVALID = mcu.MCU_trsync.REASON_COMMS_TIMEOUT + 4 def __init__( self, @@ -647,6 +658,8 @@ class LoadCellPrimitives: "math overflow", McuLoadCellProbe.ERROR_WATCHDOG: "Load Cell Probe Error: timed out " "waiting for sensor data", + McuLoadCellProbe.ERROR_ADC_INVALID: "Load Cell Probe Error: invalid " + "HX711 sample; see sensor fault diagnostics", } def __init__( diff --git a/klippy/extras/load_cell/sos_filter.py b/klippy/extras/load_cell/sos_filter.py index 13a014d3fb..3f63400b73 100644 --- a/klippy/extras/load_cell/sos_filter.py +++ b/klippy/extras/load_cell/sos_filter.py @@ -10,8 +10,8 @@ from klippy.mcu import MCU -MAX_INT32 = 2**31 -MIN_INT32 = -(2**31) - 1 +MAX_INT32 = 2**31 - 1 +MIN_INT32 = -(2**31) def assert_is_int32(value: int, error: str) -> int: diff --git a/src/load_cell_probe.c b/src/load_cell_probe.c index 8f2a3dff3f..5e48a4c5ed 100644 --- a/src/load_cell_probe.c +++ b/src/load_cell_probe.c @@ -36,6 +36,8 @@ typedef int64_t fixedQ48_t; #define ERROR_SAFETY_RANGE 0 #define ERROR_OVERFLOW 1 #define ERROR_WATCHDOG 2 +#define ERROR_ADC_INVALID 3 +#define TRIGGER_CONFIRM_SAMPLES 2 // Flags enum {FLAG_IS_HOMING = 1 << 0 @@ -51,7 +53,7 @@ struct load_cell_probe { struct trsync *ts; int32_t safety_counts_min, safety_counts_max, tare_counts; uint8_t flags, trigger_reason, error_reason, watchdog_max - , watchdog_count; + , watchdog_count, trigger_count; fixedQ16_t trigger_grams_fixed; fixedQ2_t grams_per_count; struct sos_filter *sf; @@ -163,9 +165,26 @@ load_cell_probe_report_sample(struct load_cell_probe *lcp const fixedQ16_t filtered_grams = sosfilt(lcp->sf, (fixedQ16_t)raw_grams); // update trigger state - if (abs(filtered_grams) >= lcp->trigger_grams_fixed) { - try_trigger(lcp, lcp->last_sample_ticks); + int64_t magnitude = filtered_grams; + if (magnitude < 0) + magnitude = -magnitude; + if (magnitude < lcp->trigger_grams_fixed) { + lcp->trigger_count = 0; + return; } + // Two consecutive qualified, filtered samples prevent a remaining + // one-frame impulse from ending a move. At 80 SPS this adds <=12.5ms. + if (lcp->trigger_count < TRIGGER_CONFIRM_SAMPLES) + lcp->trigger_count++; + if (lcp->trigger_count >= TRIGGER_CONFIRM_SAMPLES) + try_trigger(lcp, lcp->last_sample_ticks); +} + +void +load_cell_probe_report_fault(struct load_cell_probe *lcp) +{ + if (is_flag_set(FLAG_IS_HOMING, lcp)) + trigger_error(lcp, ERROR_ADC_INVALID); } // Timer callback that monitors for timeouts @@ -226,6 +245,7 @@ command_config_load_cell_probe(uint32_t *args) lcp->trigger_ticks = 0; lcp->watchdog_max = 0; lcp->watchdog_count = 0; + lcp->trigger_count = 0; lcp->sf = sos_filter_oid_lookup(args[1]); set_endstop_range(lcp, 0, 0, 0, 0, 0); } @@ -275,6 +295,7 @@ command_load_cell_probe_home(uint32_t *args) lcp->rest_ticks = args[5]; lcp->watchdog_max = args[6]; lcp->watchdog_count = 0; + lcp->trigger_count = 0; lcp->time.func = watchdog_event; set_flag(FLAG_IS_HOMING, lcp); set_flag(FLAG_AWAIT_HOMING, lcp); diff --git a/src/load_cell_probe.h b/src/load_cell_probe.h index e67c16e55d..745fb29c80 100644 --- a/src/load_cell_probe.h +++ b/src/load_cell_probe.h @@ -6,5 +6,6 @@ struct load_cell_probe *load_cell_probe_oid_lookup(uint8_t oid); void load_cell_probe_report_sample(struct load_cell_probe *lce , int32_t sample); +void load_cell_probe_report_fault(struct load_cell_probe *lce); #endif // load_cell_probe.h diff --git a/src/sensor_hx711s.c b/src/sensor_hx711s.c index a5f64de6c4..b03d2e3485 100644 --- a/src/sensor_hx711s.c +++ b/src/sensor_hx711s.c @@ -16,29 +16,39 @@ #include #define MAX_SENSORS 4 -#define BYTES_PER_SAMPLE 4 -#define SAMPLE_ERROR_DESYNC (1L << 31) -#define SAMPLE_ERROR_READ_TOO_LONG (1L << 30) -#define SAMPLE_ERROR_TORN_READ (1L << 29) +#define BYTES_PER_VALUE 4 +#define POWERDOWN_US 10000 +#define STARTUP_DELAY_US 50000 +#define PULSE_TIME_TICKS timer_from_us(1) +#define SETTLING_FRAMES 5 +#define QUALIFY_FRAMES 2 #define HX711S_OVERFLOW (1 << 1) -// A corrupt chip frame can pass the framing checks yet carry a value -// hundreds of thousands of counts from the truth. Real contact force moves -// the summed counts by well under this between samples (a 2mm/s approach at -// 80 SPS), so a larger single-sample jump is a glitch, not a load change. -#define SPIKE_SUM_THRESHOLD 200000 +// A frame carries raw values even when it is invalid. The host must never +// feed a non-zero quality frame into a tare, filter, or probe decision. +#define HX711S_Q_SETTLING (1 << 0) +#define HX711S_Q_NOT_READY (1 << 1) +#define HX711S_Q_EXTRA_LOW (1 << 2) +#define HX711S_Q_POST_READ_LOW (1 << 3) +#define HX711S_Q_READ_OVERRUN (1 << 4) +#define HX711S_Q_SATURATED (1 << 5) +#define HX711S_Q_CHANNEL_SHIFT 8 + +enum hx711s_state { + HX711S_OFF, + HX711S_RESET, + HX711S_WAIT_READY, + HX711S_SETTLING, + HX711S_QUALIFY, + HX711S_ONLINE, +}; struct hx711s_adc { struct timer timer; uint32_t rest_ticks; - uint32_t last_error; - int32_t last_good_sum; // summed counts of the last non-glitch sample - uint8_t have_last_sum; // false until the first good sum is seen - uint8_t pending_flag; - uint8_t sensor_count; - uint8_t gain_channel; // extra clock pulses: chip type + gain selection - uint8_t sample_bytes; // sensor_count * BYTES_PER_SAMPLE - struct gpio_in sdos[MAX_SENSORS]; + uint8_t pending_flag, sensor_count, gain_channel, sample_bytes; + uint8_t state, settling_frames, qualify_frames; + struct gpio_in sdos[MAX_SENSORS]; struct gpio_out clks[MAX_SENSORS]; struct sensor_bulk sb; struct load_cell_probe *lce; @@ -46,19 +56,6 @@ struct hx711s_adc { static struct task_wake wake_hx711s; - -/**************************************************************** - * Low-level bit-banging - ****************************************************************/ - -#define MIN_PULSE_TIME nsecs_to_ticks(200) - -static uint32_t -nsecs_to_ticks(uint32_t ns) -{ - return timer_from_us(ns * 1000) / 1000000; -} - static void hx711s_delay_noirq(void) { @@ -66,7 +63,7 @@ hx711s_delay_noirq(void) asm("nop\n nop"); return; } - uint32_t end = timer_read_time() + MIN_PULSE_TIME; + uint32_t end = timer_read_time() + PULSE_TIME_TICKS; while (timer_is_before(timer_read_time(), end)) ; } @@ -76,163 +73,169 @@ hx711s_delay(void) { if (CONFIG_MACH_AVR) return; - uint32_t end = timer_read_time() + MIN_PULSE_TIME; + uint32_t end = timer_read_time() + PULSE_TIME_TICKS; while (timer_is_before(timer_read_time(), end)) irq_poll(); } -// Read num_bits from all configured chips in lockstep. All SCKs toggle -// together so each chip's post-read analog conversion starts at the same -// instant and sees no further SCK activity during its sample window. +static uint8_t +hx711s_ready_mask(struct hx711s_adc *h) +{ + uint8_t mask = 0; + for (uint8_t i = 0; i < h->sensor_count; i++) + if (gpio_in_read(h->sdos[i])) + mask |= 1 << i; + return mask; +} + +static void +hx711s_set_clocks(struct hx711s_adc *h, uint8_t value) +{ + for (uint8_t i = 0; i < h->sensor_count; i++) + gpio_out_write(h->clks[i], value); +} + +// Read all devices in lockstep. SCK is high only while IRQs are masked; the +// low phase deliberately services pending IRQs before sampling stable DOUT. static void -hx711s_raw_read(struct hx711s_adc *h, uint32_t *bits_out, int num_bits) +hx711s_raw_read(struct hx711s_adc *h, uint32_t *bits_out, uint8_t num_bits) { - uint8_t n = h->sensor_count; - for (uint8_t i = 0; i < n; i++) + for (uint8_t i = 0; i < h->sensor_count; i++) bits_out[i] = 0; while (num_bits--) { irq_disable(); - for (uint8_t i = 0; i < n; i++) - gpio_out_toggle_noirq(h->clks[i]); + hx711s_set_clocks(h, 1); hx711s_delay_noirq(); - for (uint8_t i = 0; i < n; i++) - gpio_out_toggle_noirq(h->clks[i]); - for (uint8_t i = 0; i < n; i++) - bits_out[i] = (bits_out[i] << 1) | gpio_in_read(h->sdos[i]); + hx711s_set_clocks(h, 0); irq_enable(); hx711s_delay(); + for (uint8_t i = 0; i < h->sensor_count; i++) + bits_out[i] = (bits_out[i] << 1) | gpio_in_read(h->sdos[i]); } } - -/**************************************************************** - * HX711S sensor support - ****************************************************************/ - -static uint_fast8_t -hx711s_is_data_ready(struct hx711s_adc *h) -{ - // All chips must have DOUT low before reading; they share a RATE pin but - // may not be perfectly phase-aligned, so check every chip. - for (uint8_t i = 0; i < h->sensor_count; i++) { - if (gpio_in_read(h->sdos[i])) - return 0; - } - return 1; -} - static uint_fast8_t hx711s_event(struct timer *timer) { struct hx711s_adc *h = container_of(timer, struct hx711s_adc, timer); - uint32_t rest_ticks = h->rest_ticks; + if (h->state == HX711S_OFF) + return SF_DONE; + if (h->state == HX711S_RESET) { + hx711s_set_clocks(h, 0); + h->state = HX711S_WAIT_READY; + h->timer.waketime += timer_from_us(STARTUP_DELAY_US); + return SF_RESCHEDULE; + } if (h->pending_flag) { h->sb.possible_overflows++; h->pending_flag |= HX711S_OVERFLOW; - rest_ticks *= 4; - } else if (hx711s_is_data_ready(h)) { + h->timer.waketime += h->rest_ticks * 4; + return SF_RESCHEDULE; + } + if (!hx711s_ready_mask(h)) { + if (h->state == HX711S_WAIT_READY) + h->state = HX711S_SETTLING; h->pending_flag = 1; sched_wake_task(&wake_hx711s); - rest_ticks *= 8; + h->timer.waketime += h->rest_ticks * 8; + } else { + h->timer.waketime += h->rest_ticks; } - h->timer.waketime += rest_ticks; return SF_RESCHEDULE; } static void -append_sample(struct hx711s_adc *h, int32_t val) +append_value(struct hx711s_adc *h, uint32_t value) { - h->sb.data[h->sb.data_count] = val; - h->sb.data[h->sb.data_count + 1] = val >> 8; - h->sb.data[h->sb.data_count + 2] = val >> 16; - h->sb.data[h->sb.data_count + 3] = val >> 24; - h->sb.data_count += BYTES_PER_SAMPLE; + h->sb.data[h->sb.data_count] = value; + h->sb.data[h->sb.data_count + 1] = value >> 8; + h->sb.data[h->sb.data_count + 2] = value >> 16; + h->sb.data[h->sb.data_count + 3] = value >> 24; + h->sb.data_count += BYTES_PER_VALUE; } static void -hx711s_read_adc(struct hx711s_adc *h, uint8_t oid) +hx711s_begin_reset(struct hx711s_adc *h) { - uint_fast8_t gain_channel = h->gain_channel; - uint_fast8_t extras_mask = (1 << gain_channel) - 1; - int32_t counts_buf[MAX_SENSORS]; - uint32_t adc[MAX_SENSORS]; - uint32_t sample_error = 0; - - // The chips free-run on independent oscillators, so a chip may latch a - // new conversion into its output register between the data-ready poll - // and this read, or during the read itself. Bits clocked out across a - // latch are torn (often all-ones) with no other error signature. - // Detect this by verifying every DOUT is still low immediately before - // the read and back high immediately after; a low DOUT after the read - // means that chip latched new data mid-read. - uint32_t torn_read = !hx711s_is_data_ready(h); + h->pending_flag = 0; + hx711s_set_clocks(h, 1); + h->state = HX711S_RESET; + h->settling_frames = SETTLING_FRAMES; + h->qualify_frames = QUALIFY_FRAMES; + h->timer.waketime = timer_read_time() + timer_from_us(POWERDOWN_US); + sched_add_timer(&h->timer); +} - hx711s_raw_read(h, adc, 24 + gain_channel); +static void +hx711s_read_adc(struct hx711s_adc *h, uint8_t oid) +{ + uint32_t adc[MAX_SENSORS], quality = 0; + int32_t counts[MAX_SENSORS]; + uint8_t channel_mask = hx711s_ready_mask(h); + uint8_t extras_mask = (1 << h->gain_channel) - 1; + if (channel_mask) + quality |= HX711S_Q_NOT_READY; + hx711s_delay(); + hx711s_raw_read(h, adc, 24 + h->gain_channel); hx711s_delay(); - for (uint8_t i = 0; i < h->sensor_count; i++) { - if (!gpio_in_read(h->sdos[i])) - torn_read = 1; - } for (uint8_t i = 0; i < h->sensor_count; i++) { - uint32_t raw = adc[i] >> gain_channel; + uint32_t raw = adc[i] >> h->gain_channel; if (raw & 0x800000) - raw |= 0xFF000000; - counts_buf[i] = (int32_t)raw; - if ((adc[i] & extras_mask) != extras_mask) - sample_error = SAMPLE_ERROR_DESYNC; + raw |= 0xff000000; + counts[i] = raw; + if ((adc[i] & extras_mask) != extras_mask) { + quality |= HX711S_Q_EXTRA_LOW; + channel_mask |= 1 << i; + } + // After the final gain-selection pulse DOUT must remain high until + // the next conversion. A low line is a protocol/electrical fault, + // not a sample to silently hold or synthesize. + if (!gpio_in_read(h->sdos[i])) { + quality |= HX711S_Q_POST_READ_LOW; + channel_mask |= 1 << i; + } + if (counts[i] == INT32_C(0x007fffff) + || counts[i] == -INT32_C(0x00800000)) { + quality |= HX711S_Q_SATURATED; + channel_mask |= 1 << i; + } } - // Capture and clear pending flag after all reads irq_disable(); uint8_t flags = h->pending_flag; h->pending_flag = 0; irq_enable(); - if (flags & HX711S_OVERFLOW) - sample_error = SAMPLE_ERROR_READ_TOO_LONG; - - // Torn reads are a normal consequence of chip oscillator drift: discard - // just that sample, the next conversion is valid. They can also corrupt - // the extra-bit check, so they must take priority over a desync. A real - // desync corrupts the serial protocol state, making all further values - // unreliable until the chips are power cycled: latch it and keep - // reporting it so the host restarts the sensor. - if (torn_read) - sample_error = SAMPLE_ERROR_TORN_READ; - else if (sample_error) - h->last_error = sample_error; + quality |= HX711S_Q_READ_OVERRUN; + + if (h->state == HX711S_SETTLING || h->state == HX711S_QUALIFY) { + if (!quality && h->state == HX711S_SETTLING && !--h->settling_frames) + h->state = HX711S_QUALIFY; + else if (!quality && h->state == HX711S_QUALIFY && !--h->qualify_frames) + h->state = HX711S_ONLINE; + quality |= HX711S_Q_SETTLING; + } + quality |= (uint32_t)channel_mask << HX711S_Q_CHANNEL_SHIFT; - if (h->last_error) - sample_error = h->last_error; + for (uint8_t i = 0; i < h->sensor_count; i++) + append_value(h, counts[i]); + append_value(h, quality); - if (sample_error) { - for (uint8_t i = 0; i < h->sensor_count; i++) - append_sample(h, (int32_t)sample_error); - } else { + if (!quality && h->state == HX711S_ONLINE) { int32_t sum = 0; - for (uint8_t i = 0; i < h->sensor_count; i++) { - append_sample(h, counts_buf[i]); - sum += counts_buf[i]; - } - // Reject single-sample glitches that pass framing but jump - // implausibly far from the last good reading - these would - // otherwise fire a false probe trigger. Only the trigger is - // suppressed; the raw value still went into the bulk buffer above - // so the host can see and hold it (and log which chip glitched). - int32_t jump = sum - h->last_good_sum; - if (jump < 0) - jump = -jump; - if (!h->have_last_sum || jump <= SPIKE_SUM_THRESHOLD) { - h->last_good_sum = sum; - h->have_last_sum = 1; - if (h->lce) - load_cell_probe_report_sample(h->lce, sum); - } + for (uint8_t i = 0; i < h->sensor_count; i++) + sum += counts[i]; + if (h->lce) + load_cell_probe_report_sample(h->lce, sum); + } else if (quality && h->lce) { + load_cell_probe_report_fault(h->lce); } - // Flush buffer if another sample would overflow it + if (quality & (HX711S_Q_NOT_READY | HX711S_Q_EXTRA_LOW + | HX711S_Q_POST_READ_LOW | HX711S_Q_READ_OVERRUN)) + hx711s_begin_reset(h); if (h->sb.data_count + h->sample_bytes > ARRAY_SIZE(h->sb.data)) sensor_bulk_report(&h->sb, oid); } @@ -243,15 +246,14 @@ command_config_hx711s(uint32_t *args) struct hx711s_adc *h = oid_alloc(args[0], command_config_hx711s, sizeof(*h)); h->timer.func = hx711s_event; - uint8_t sensor_count = args[1]; - if (sensor_count < 1 || sensor_count > MAX_SENSORS) + if (args[1] < 1 || args[1] > MAX_SENSORS) shutdown("hx711s: sensor_count must be 1-4"); - h->sensor_count = sensor_count; - uint8_t gain_channel = args[2]; - if (gain_channel < 1 || gain_channel > 4) + if (args[2] < 1 || args[2] > 4) shutdown("hx711s: gain_channel out of range 1-4"); - h->gain_channel = gain_channel; - h->sample_bytes = BYTES_PER_SAMPLE * sensor_count; + h->sensor_count = args[1]; + h->gain_channel = args[2]; + h->sample_bytes = BYTES_PER_VALUE * (h->sensor_count + 1); + h->state = HX711S_OFF; } DECL_COMMAND(command_config_hx711s, "config_hx711s oid=%c sensor_count=%c gain_channel=%c"); @@ -259,14 +261,11 @@ DECL_COMMAND(command_config_hx711s, void command_add_hx711s(uint32_t *args) { - uint8_t oid = args[0]; - struct hx711s_adc *h = oid_lookup(oid, command_config_hx711s); - uint8_t index = args[1]; - if (index >= h->sensor_count) + struct hx711s_adc *h = oid_lookup(args[0], command_config_hx711s); + if (args[1] >= h->sensor_count) shutdown("hx711s: sensor index out of range"); - h->sdos[index] = gpio_in_setup(args[2], 1); - h->clks[index] = gpio_out_setup(args[3], 0); - gpio_out_write(h->clks[index], 1); // put chip in power down state + h->sdos[args[1]] = gpio_in_setup(args[2], 1); + h->clks[args[1]] = gpio_out_setup(args[3], 1); } DECL_COMMAND(command_add_hx711s, "add_hx711s oid=%c index=%c sdo_pin=%u sclk_pin=%u"); @@ -274,8 +273,7 @@ DECL_COMMAND(command_add_hx711s, void hx711s_attach_load_cell_probe(uint32_t *args) { - uint8_t oid = args[0]; - struct hx711s_adc *h = oid_lookup(oid, command_config_hx711s); + struct hx711s_adc *h = oid_lookup(args[0], command_config_hx711s); h->lce = load_cell_probe_oid_lookup(args[1]); } DECL_COMMAND(hx711s_attach_load_cell_probe, @@ -284,42 +282,26 @@ DECL_COMMAND(hx711s_attach_load_cell_probe, void command_query_hx711s(uint32_t *args) { - uint8_t oid = args[0]; - struct hx711s_adc *h = oid_lookup(oid, command_config_hx711s); + struct hx711s_adc *h = oid_lookup(args[0], command_config_hx711s); sched_del_timer(&h->timer); - h->pending_flag = 0; - h->last_error = 0; - h->have_last_sum = 0; h->rest_ticks = args[1]; if (!h->rest_ticks) { - for (uint8_t i = 0; i < h->sensor_count; i++) - gpio_out_write(h->clks[i], 1); // power down all chips + hx711s_set_clocks(h, 1); + h->state = HX711S_OFF; return; } - for (uint8_t i = 0; i < h->sensor_count; i++) - gpio_out_write(h->clks[i], 0); // wake all chips sensor_bulk_reset(&h->sb); - // HX711 needs up to ~400ms (typ. 10-20ms) to settle after PD release. - // Wait 50ms before first poll so the first DRDY isn't from a still- - // stabilising chip (which can produce a spurious DESYNC). - irq_disable(); - h->timer.waketime = timer_read_time() + timer_from_us(50000); - sched_add_timer(&h->timer); - irq_enable(); + hx711s_begin_reset(h); } DECL_COMMAND(command_query_hx711s, "query_hx711s oid=%c rest_ticks=%u"); void command_query_hx711s_status(const uint32_t *args) { - uint8_t oid = args[0]; - struct hx711s_adc *h = oid_lookup(oid, command_config_hx711s); - irq_disable(); - const uint32_t start_t = timer_read_time(); - uint8_t is_ready = hx711s_is_data_ready(h); - irq_enable(); - uint8_t pending_bytes = is_ready ? h->sample_bytes : 0; - sensor_bulk_status(&h->sb, oid, start_t, 0, pending_bytes); + struct hx711s_adc *h = oid_lookup(args[0], command_config_hx711s); + uint32_t start_t = timer_read_time(); + uint8_t pending_bytes = !hx711s_ready_mask(h) ? h->sample_bytes : 0; + sensor_bulk_status(&h->sb, args[0], start_t, 0, pending_bytes); } DECL_COMMAND(command_query_hx711s_status, "query_hx711s_status oid=%c"); diff --git a/src/sos_filter.c b/src/sos_filter.c index 3ec5d6178b..b2f416c2ea 100644 --- a/src/sos_filter.c +++ b/src/sos_filter.c @@ -70,13 +70,20 @@ sosfilt(struct sos_filter *sf, const int32_t unfiltered_value) { for (int section_idx = 0; section_idx < sf->n_sections; section_idx++) { struct sos_filter_section *section = &(sf->filter[section_idx]); // apply the section's filter coefficients to input - fixedQ_value_t next_val = fixed_mul(sf, section->coeff[0], cur_val); - next_val += section->state[0]; - section->state[0] = fixed_mul(sf, section->coeff[1], cur_val) - - fixed_mul(sf, section->coeff[3], next_val) - + (section->state[1]); - section->state[1] = fixed_mul(sf, section->coeff[2], cur_val) - - fixed_mul(sf, section->coeff[4], next_val); + int64_t next = (int64_t)fixed_mul(sf, section->coeff[0], cur_val) + + section->state[0]; + if (overflows_int32(next)) + shutdown("sosfilt: output overflow"); + fixedQ_value_t next_val = next; + int64_t state0 = (int64_t)fixed_mul(sf, section->coeff[1], cur_val) + - fixed_mul(sf, section->coeff[3], next_val) + + section->state[1]; + int64_t state1 = (int64_t)fixed_mul(sf, section->coeff[2], cur_val) + - fixed_mul(sf, section->coeff[4], next_val); + if (overflows_int32(state0) || overflows_int32(state1)) + shutdown("sosfilt: state overflow"); + section->state[0] = state0; + section->state[1] = state1; cur_val = next_val; } @@ -108,8 +115,8 @@ sos_filter_oid_lookup(uint8_t oid) static void validate_section_index(struct sos_filter *sf, uint8_t section_idx) { - if (section_idx > sf->max_sections) - shutdown("Filter section index larger than max_sections"); + if (section_idx >= sf->max_sections) + shutdown("Filter section index outside max_sections"); } // Set one section of the filter @@ -154,7 +161,8 @@ command_sos_filter_activate(uint32_t *args) { struct sos_filter *sf = sos_filter_oid_lookup(args[0]); uint8_t n_sections = args[1]; - validate_section_index(sf, n_sections); + if (n_sections > sf->max_sections) + shutdown("Filter section count larger than max_sections"); sf->n_sections = n_sections; const uint8_t coeff_int_bits = args[2]; sf->coeff_frac_bits = (31 - coeff_int_bits); From 9ed1574a997a331a332cba71f8d75ce21d3b1f63 Mon Sep 17 00:00:00 2001 From: Paul Swenson <1937248+pdscomp@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:07:07 -0400 Subject: [PATCH 02/11] hx711s: flush bulk buffer before appending a frame A four-channel HX711 frame occupies 20 bytes while the shared sensor_bulk buffer holds 51 bytes. After two frames, appending another before checking capacity writes past the end of the buffer and can corrupt MCU state. Flush an existing packet before appending a frame that would not fit. --- src/sensor_hx711s.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/sensor_hx711s.c b/src/sensor_hx711s.c index b03d2e3485..d23e4c2790 100644 --- a/src/sensor_hx711s.c +++ b/src/sensor_hx711s.c @@ -219,6 +219,12 @@ hx711s_read_adc(struct hx711s_adc *h, uint8_t oid) } quality |= (uint32_t)channel_mask << HX711S_Q_CHANNEL_SHIFT; + // A complete sample must fit before appending it. In the four-channel + // case each frame is 20 bytes, while the shared bulk buffer is 51 bytes: + // appending first when it already contains 40 bytes would overrun it. + if (h->sb.data_count + h->sample_bytes > ARRAY_SIZE(h->sb.data)) + sensor_bulk_report(&h->sb, oid); + for (uint8_t i = 0; i < h->sensor_count; i++) append_value(h, counts[i]); append_value(h, quality); @@ -236,8 +242,6 @@ hx711s_read_adc(struct hx711s_adc *h, uint8_t oid) if (quality & (HX711S_Q_NOT_READY | HX711S_Q_EXTRA_LOW | HX711S_Q_POST_READ_LOW | HX711S_Q_READ_OVERRUN)) hx711s_begin_reset(h); - if (h->sb.data_count + h->sample_bytes > ARRAY_SIZE(h->sb.data)) - sensor_bulk_report(&h->sb, oid); } void From 28aca926429211037556af2cd2d4c2ae94607b8b Mon Sep 17 00:00:00 2001 From: Paul Swenson <1937248+pdscomp@users.noreply.github.com> Date: Wed, 22 Jul 2026 20:13:11 -0400 Subject: [PATCH 03/11] hx711s: reschedule timer safely during recovery Frame faults are handled from task context while the polling timer remains linked in the MCU scheduler. Adding that same timer again corrupts the timer list and eventually manifests as serial garbage, disconnects, or Invalid oid type shutdowns. Remove the timer before changing its wake time and re-adding it, and use the same reset path for command-started acquisition. --- src/sensor_hx711s.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/sensor_hx711s.c b/src/sensor_hx711s.c index d23e4c2790..1b155a02f8 100644 --- a/src/sensor_hx711s.c +++ b/src/sensor_hx711s.c @@ -157,6 +157,10 @@ append_value(struct hx711s_adc *h, uint32_t value) static void hx711s_begin_reset(struct hx711s_adc *h) { + // A runtime frame fault reaches here from task context while the polling + // timer is still linked. Re-adding that timer corrupts the scheduler + // list, so remove it before changing its wake time and inserting it. + sched_del_timer(&h->timer); h->pending_flag = 0; hx711s_set_clocks(h, 1); h->state = HX711S_RESET; @@ -287,9 +291,9 @@ void command_query_hx711s(uint32_t *args) { struct hx711s_adc *h = oid_lookup(args[0], command_config_hx711s); - sched_del_timer(&h->timer); h->rest_ticks = args[1]; if (!h->rest_ticks) { + sched_del_timer(&h->timer); hx711s_set_clocks(h, 1); h->state = HX711S_OFF; return; From 9044d16e49295c3c405601a3818c390a9951b213 Mon Sep 17 00:00:00 2001 From: Paul Swenson <1937248+pdscomp@users.noreply.github.com> Date: Wed, 22 Jul 2026 21:35:11 -0400 Subject: [PATCH 04/11] hx711s: harden recovery, timing, and probe fault handling Make the multi-channel HX711 path fail closed on invalid frames and recover without corrupting MCU state. Timestamp each frame, validate the wire format, honor HX711 ready-line timing, close stop and scheduler races, and latch terminal probe outcomes. Preserve timestamped fault details through Kalico, exclude recovery frames from tare failures, use summed channel ranges, and expose health counters. Add rate-independent trigger confirmation, focused regression tests, and documentation for the new protocol and diagnostics. --- docs/Config_Reference.md | 5 + docs/G-Codes.md | 2 + docs/Status_Reference.md | 3 + klippy/extras/load_cell/hx711s.py | 253 +++++++++++++++++++-- klippy/extras/load_cell/interfaces.py | 4 + klippy/extras/load_cell/load_cell.py | 36 ++- klippy/extras/load_cell/load_cell_probe.py | 41 +++- src/load_cell_probe.c | 124 ++++++---- src/load_cell_probe.h | 7 +- src/sensor_hx711s.c | 133 +++++++---- src/sos_filter.c | 2 + test/test_hx711_reliability.py | 204 +++++++++++++++++ 12 files changed, 683 insertions(+), 131 deletions(-) create mode 100644 test/test_hx711_reliability.py diff --git a/docs/Config_Reference.md b/docs/Config_Reference.md index 815ec11819..f29f5747cf 100644 --- a/docs/Config_Reference.md +++ b/docs/Config_Reference.md @@ -6141,6 +6141,11 @@ sensor_type: # The default is +/-1Kg. #trigger_force: 75.0 # The force that the probe will trigger at. 75g is the default. +#trigger_confirm_time: 0.0125 +# The minimum time, in seconds, that force must remain above trigger_force +# before a probe is accepted. The first threshold crossing remains the +# reported contact time. The default is 0.0125 seconds. Set to 0 to disable +# confirmation. #drift_filter_cutoff_frequency: 0.8 # Enable optional continuous taring while homing & probing to reject drift. # The value is a frequency, in Hz, below which drift will be ignored. This diff --git a/docs/G-Codes.md b/docs/G-Codes.md index a0a72f70cb..93c0857ff7 100644 --- a/docs/G-Codes.md +++ b/docs/G-Codes.md @@ -1084,6 +1084,7 @@ corresponding settings from the [`[load_cell_probe]`](./Config_Reference.md#load_cell_probe) configuration: - `FORCE_SAFETY_LIMIT=` - `TRIGGER_FORCE=` +- `TRIGGER_CONFIRM_TIME=` - `DRIFT_FILTER_CUTOFF_FREQUENCY=` - `DRIFT_FILTER_DELAY=<1|2>` - `BUZZ_FILTER_CUTOFF_FREQUENCY=` @@ -1091,6 +1092,7 @@ corresponding settings from the - `NOTCH_FILTER_FREQUENCIES=` - `NOTCH_FILTER_QUALITY=` - `TARE_TIME=` +- `TARE_SAMPLES=` - `PULLBACK_DISTANCE=` - `PULLBACK_SPEED=` - `MIN_TAP_QUALITY=` diff --git a/docs/Status_Reference.md b/docs/Status_Reference.md index fcd41cd7d4..ca18ec5a5c 100644 --- a/docs/Status_Reference.md +++ b/docs/Status_Reference.md @@ -367,6 +367,9 @@ The following information is available for each `[load_cell name]`: - 'force_g_per_channel': The force in grams, averaged over the last polling period, one entry per channel - 'min_force_g': The minimum force in grams, over the last polling period. - 'max_force_g': The maximum force in grams, over the last polling period. +- 'sensor_health': For sensors that report acquisition health, cumulative + frame, quality-flag, and per-channel fault counters together with the latest + hard fault and latest batch summary. ## load_cell_probe diff --git a/klippy/extras/load_cell/hx711s.py b/klippy/extras/load_cell/hx711s.py index 95b262fd10..b8d72faa66 100644 --- a/klippy/extras/load_cell/hx711s.py +++ b/klippy/extras/load_cell/hx711s.py @@ -6,15 +6,136 @@ # # This file may be distributed under the terms of the GNU GPLv3 license. import logging +import struct +from collections import Counter from klippy.mcu import MCU from .. import bulk_sensor -from .interfaces import AdcFault, BulkAdcData, BulkAdcDataCallback, LoadCellSensor +from .interfaces import ( + AdcFault, + BulkAdcData, + BulkAdcDataCallback, + LoadCellSensor, +) UPDATE_INTERVAL = 0.10 ADC_FACTOR = 1.0 / (1 << 23) +Q_SETTLING = 1 << 0 +Q_NOT_READY = 1 << 1 +Q_EXTRA_LOW = 1 << 2 +Q_POST_READ_LOW = 1 << 3 +Q_READ_OVERRUN = 1 << 4 +Q_SATURATED = 1 << 5 +Q_FRAME_FORMAT = 1 << 6 +Q_CHANNEL_SHIFT = 8 +Q_CHANNEL_MASK = 0xF << Q_CHANNEL_SHIFT +FRAME_TAG = 0xA7110000 +FRAME_TAG_MASK = 0xFFFF0000 +QUALITY_FLAGS = ( + (Q_SETTLING, "settling"), + (Q_NOT_READY, "not_ready"), + (Q_EXTRA_LOW, "extra_low"), + (Q_POST_READ_LOW, "post_read_low"), + (Q_READ_OVERRUN, "read_overrun"), + (Q_SATURATED, "saturated"), + (Q_FRAME_FORMAT, "frame_format"), +) + + +def quality_is_hard(quality: int) -> bool: + # Channel bits and unknown future bits are conservatively hard. The only + # non-control frame that is expected during successful recovery is a pure + # settling/qualification frame. + return bool(quality & ~Q_SETTLING) + + +def quality_flags(quality: int) -> tuple[str, ...]: + flags = tuple(name for bit, name in QUALITY_FLAGS if quality & bit) + known = sum(bit for bit, _name in QUALITY_FLAGS) | Q_CHANNEL_MASK + unknown = quality & ~known & 0xFFFFFFFF + if unknown: + flags += (f"unknown_0x{unknown:x}",) + return flags + + +class TimestampedBulkReader: + """Read records that carry their own MCU capture clock. + + HX711 recovery deliberately creates gaps, so treating record sequence as a + continuous fixed-frequency sample clock corrupts timestamps around the + event. This reader leaves rate estimation out of the control path and + uses the timestamp embedded in every MCU frame instead. + """ + + def __init__(self, mcu, unpack_fmt): + self.mcu = mcu + self.unpack = struct.Struct(unpack_fmt) + self.bulk_queue = None + self.query_status_cmd = None + self.oid = None + self.last_overflows = 0 + self._mcu_overflows = 0 + self._next_sequence = 0 + + def setup_query_command(self, msgformat, oid, cq): + self.oid = oid + self.query_status_cmd = self.mcu.lookup_query_command( + msgformat, + "sensor_bulk_status oid=%c clock=%u query_ticks=%u" + " next_sequence=%hu buffered=%u possible_overflows=%hu", + oid=oid, + cq=cq, + ) + self.bulk_queue = bulk_sensor.BulkDataQueue(self.mcu, oid=oid) + + def get_last_overflows(self): + return self.last_overflows + + def _update_status(self): + params = self.query_status_cmd.send([self.oid]) + overflows = params["possible_overflows"] + delta = (overflows - self._mcu_overflows) & 0xFFFF + self._mcu_overflows = overflows + self.last_overflows += delta + return params + + def note_start(self): + self.bulk_queue.clear_queue() + self.last_overflows = 0 + params = self.query_status_cmd.send([self.oid]) + self._mcu_overflows = params["possible_overflows"] + self._next_sequence = params["next_sequence"] + + def note_end(self): + self.bulk_queue.clear_queue() + + def pull_samples(self): + self._update_status() + samples = [] + for params in self.bulk_queue.pull_queue(): + sequence = params["sequence"] + missing = (sequence - self._next_sequence) & 0xFFFF + if missing: + logging.error( + "HX711 bulk stream skipped %d message(s)", missing + ) + self.last_overflows += missing + self._next_sequence = (sequence + 1) & 0xFFFF + data = params["data"] + if len(data) % self.unpack.size: + logging.error( + "HX711 bulk record has %d trailing byte(s)", + len(data) % self.unpack.size, + ) + self.last_overflows += 1 + for offset in range( + 0, len(data) - self.unpack.size + 1, self.unpack.size + ): + samples.append(self.unpack.unpack_from(data, offset)) + return samples + class HX711SBase(LoadCellSensor): def __init__( @@ -30,6 +151,16 @@ def __init__( self.name = config.get_name().split()[-1] self.sensor_type = sensor_type self.consecutive_fails = 0 + self._health = Counter() + self._quality_totals = Counter() + self._channel_fault_totals = Counter() + self._last_hard_fault = None + self._last_batch_health = {} + self._pending_log_flags = Counter() + self._pending_log_channels = Counter() + self._pending_log_hard = 0 + self._pending_log_settling = 0 + self._last_fault_log = float("-inf") ppins = self.printer.lookup_object("pins") sdo_pin_names = [p.strip() for p in config.get("sdo_pins").split(",")] @@ -68,12 +199,10 @@ def __init__( self.oid = mcu.create_oid() # Bulk sensor setup - chip_smooth = self.sps * UPDATE_INTERVAL * 2 # The final word is MCU-owned frame quality. Raw counts stay in the # stream for diagnosis, but only quality==0 reaches control clients. - unpack_format = "<" + ("i" * self.sensor_count) + "I" - self.ffreader = bulk_sensor.FixedFreqReader(mcu, chip_smooth, - unpack_format) + unpack_format = " tuple[int, int]: def get_channel_count(self) -> int: return self.sensor_count + def get_health(self): + return { + "frames": dict(self._health), + "quality_flags": dict(self._quality_totals), + "channel_faults": { + str(channel): count + for channel, count in self._channel_fault_totals.items() + }, + "last_hard_fault": self._last_hard_fault, + "last_batch": self._last_batch_health, + } + def add_client(self, callback: BulkAdcDataCallback): self.batch_bulk.add_client(callback) @@ -135,15 +276,43 @@ def _convert_samples(self, samples): count = 0 faults: list[AdcFault] = [] for sample in samples: - ptime = sample[0] + capture_clock = self.mcu.clock32_to_clock64(sample[0]) + ptime = self.mcu.clock_to_print_time(capture_clock) channel_counts = tuple(sample[1 : 1 + self.sensor_count]) - quality = sample[1 + self.sensor_count] + wire_quality = sample[1 + self.sensor_count] + if wire_quality & FRAME_TAG_MASK != FRAME_TAG: + quality = Q_FRAME_FORMAT + else: + quality = wire_quality & ~FRAME_TAG_MASK + self._health["received"] += 1 if quality: - faults.append( - {"time": round(ptime, 6), "counts": channel_counts, - "quality": quality} + channels = tuple( + channel + for channel in range(self.sensor_count) + if quality & (1 << (Q_CHANNEL_SHIFT + channel)) ) + flags = quality_flags(quality) + hard = quality_is_hard(quality) + fault: AdcFault = { + "time": round(ptime, 6), + "counts": channel_counts, + "quality": quality, + "wire_quality": wire_quality, + "flags": flags, + "channels": channels, + "hard": hard, + } + faults.append(fault) + self._health["fault"] += 1 + self._health["hard" if hard else "settling"] += 1 + for flag in flags: + self._quality_totals[flag] += 1 + for channel in channels: + self._channel_fault_totals[channel] += 1 + if hard: + self._last_hard_fault = fault continue + self._health["valid"] += 1 converted = [round(ptime, 6)] for ch in channel_counts: converted.append(ch) @@ -153,6 +322,44 @@ def _convert_samples(self, samples): del samples[count:] return faults + def _log_faults(self, eventtime, faults): + if not any(fault["hard"] for fault in faults): + return + for fault in faults: + for flag in fault["flags"]: + self._pending_log_flags[flag] += 1 + for channel in fault["channels"]: + self._pending_log_channels[channel] += 1 + if fault["hard"]: + self._pending_log_hard += 1 + else: + self._pending_log_settling += 1 + if not self._pending_log_hard or eventtime < self._last_fault_log + 1.0: + return + flag_summary = ", ".join( + f"{name}={count}" + for name, count in sorted(self._pending_log_flags.items()) + ) + channel_summary = ( + ", ".join( + f"ch{channel}={count}" + for channel, count in sorted(self._pending_log_channels.items()) + ) + or "none" + ) + logging.warning( + "%s: HX711 faults: hard=%d recovery=%d; flags=[%s]; channels=[%s]", + self.name, + self._pending_log_hard, + self._pending_log_settling, + flag_summary, + channel_summary, + ) + self._pending_log_flags.clear() + self._pending_log_channels.clear() + self._pending_log_hard = self._pending_log_settling = 0 + self._last_fault_log = eventtime + def _start_measurements(self): self.consecutive_fails = 0 rest_ticks = self.mcu.seconds_to_clock( @@ -162,34 +369,36 @@ def _start_measurements(self): logging.info( "%s starting '%s' measurements", self.sensor_type, self.name ) - self.ffreader.note_start() + self.bulk_reader.note_start() def _finish_measurements(self): if self.printer.is_shutdown(): return self.query_hx711s_cmd.send_wait_ack([self.oid, 0]) - self.ffreader.note_end() + self.bulk_reader.note_end() logging.info( "%s finished '%s' measurements", self.sensor_type, self.name ) def _process_batch(self, eventtime) -> BulkAdcData: - prev_overflows = self.ffreader.get_last_overflows() - samples = self.ffreader.pull_samples() + prev_overflows = self.bulk_reader.get_last_overflows() + samples = self.bulk_reader.pull_samples() faults = self._convert_samples(samples) - overflows = self.ffreader.get_last_overflows() - prev_overflows - errors = len(faults) - if faults: - logging.warning( - "%s: dropped %d invalid HX711 frame(s); latest quality=0x%x", - self.name, errors, faults[-1]["quality"], - ) + overflows = self.bulk_reader.get_last_overflows() - prev_overflows + errors = sum(fault["hard"] for fault in faults) + self._log_faults(eventtime, faults) if overflows > 0: self.consecutive_fails += 1 if self.consecutive_fails > 4: logging.error("%s: repeated bulk overflows", self.name) else: self.consecutive_fails = 0 + self._last_batch_health = { + "valid": len(samples), + "hard_faults": errors, + "recovery_frames": len(faults) - errors, + "bulk_overflows": overflows, + } return { "data": samples, "errors": errors, diff --git a/klippy/extras/load_cell/interfaces.py b/klippy/extras/load_cell/interfaces.py index cc256527fc..eb3a65a009 100644 --- a/klippy/extras/load_cell/interfaces.py +++ b/klippy/extras/load_cell/interfaces.py @@ -17,6 +17,10 @@ class AdcFault(TypedDict): time: float counts: tuple[int, ...] quality: int + wire_quality: int + flags: tuple[str, ...] + channels: tuple[int, ...] + hard: bool class BulkAdcData(TypedDict): diff --git a/klippy/extras/load_cell/load_cell.py b/klippy/extras/load_cell/load_cell.py index b815ddf612..d08fb9be54 100644 --- a/klippy/extras/load_cell/load_cell.py +++ b/klippy/extras/load_cell/load_cell.py @@ -441,7 +441,19 @@ def _complete(self): def _on_samples(self, msg): if not self.is_started: return False # already stopped, ignore - self._errors += msg["errors"] + faults = msg.get("faults") + if faults: + for fault in faults: + fault_time = fault["time"] + if self.min_time <= fault_time <= self.max_time: + if fault.get("hard", True): + self._errors += 1 + if fault_time > self.max_time: + self._complete() + elif msg["errors"]: + # Sensors without timestamped fault records retain the legacy + # batch-wide error behavior. + self._errors += msg["errors"] self._overflows += msg["overflows"] samples = msg["data"] for sample in samples: @@ -477,10 +489,11 @@ def _collect_until(self, timeout): self._completion = self._reactor.completion() result = self._completion.wait(waketime=wake_time) if result is None: - self._finish_collecting() + _samples, errors = self._finish_collecting() + error_count, overflow_count = errors if errors else (0, 0) raise self._printer.command_error( - f"LoadCellSampleCollector timed out! Errors: {self._errors}," - f" Overflows: {self._overflows}" + f"LoadCellSampleCollector timed out! Errors: " + f"{error_count}, Overflows: {overflow_count}" ) return self._finish_collecting() @@ -661,6 +674,7 @@ def _sensor_data_event(self, msg): data = msg.get("data") errors = msg.get("errors") overflows = msg.get("overflows") + faults = msg.get("faults", []) if data is None: return None samples = [] @@ -688,8 +702,10 @@ def _sensor_data_event(self, msg): sample.append(counts) samples.append(sample) msg = { - "data": samples, "errors": errors, "overflows": overflows, - "faults": [], + "data": samples, + "errors": errors, + "overflows": overflows, + "faults": faults, } self.clients.send(msg) return True @@ -816,9 +832,10 @@ def avg_counts(self, num_samples: int | None = None) -> tuple[int, ...]: def validate_samples(self, samples, errors): if errors: + error_count, overflow_count = errors raise self.printer.command_error( - "Sensor reported %i errors while sampling" - % (errors[0] + errors[1]) + "Sensor reported %i acquisition errors and %i bulk " + "overflows while sampling" % (error_count, overflow_count) ) # check individual channels for saturated readings range_min, range_max = self.channel_saturation_range() @@ -909,4 +926,7 @@ def get_status(self, eventtime): "tare_force": self.tare_force, } ) + get_health = getattr(self.sensor, "get_health", None) + if get_health is not None: + status["sensor_health"] = get_health() return status diff --git a/klippy/extras/load_cell/load_cell_probe.py b/klippy/extras/load_cell/load_cell_probe.py index 7dc553c9a0..d2aaf143de 100644 --- a/klippy/extras/load_cell/load_cell_probe.py +++ b/klippy/extras/load_cell/load_cell_probe.py @@ -405,6 +405,17 @@ def __init__( self._trigger_force_param = intParamHelper( config, "trigger_force", default=75, minval=10, maxval=250 ) + # Require force to remain above the threshold for a fixed duration, + # independent of the ADC's configured sample rate. The MCU reports + # the first crossing as contact time after the later samples confirm + # it was not a one-frame impulse. + self._trigger_confirm_time_param = floatParamHelper( + config, + "trigger_confirm_time", + default=0.0125, + minval=0.0, + maxval=0.100, + ) self._force_safety_limit_param = intParamHelper( config, "force_safety_limit", minval=0, default=2000 ) @@ -439,6 +450,9 @@ def get_tare_samples(self, gcmd=None) -> int: def get_trigger_force_grams(self, gcmd=None) -> int: return self._trigger_force_param.get(gcmd) + def get_trigger_confirm_time(self, gcmd=None) -> float: + return self._trigger_confirm_time_param.get(gcmd) + def get_safety_limit_grams(self, gcmd=None) -> int: return self._force_safety_limit_param.get(gcmd) @@ -472,7 +486,7 @@ def get_reference_safety_range(self, gcmd=None) -> Tuple[int, int]: safety_min = int(zero - safety_counts) safety_max = int(zero + safety_counts) # don't allow a safety range outside the sensor's real range - sensor_min, sensor_max = self._load_cell.get_sensor().get_range() + sensor_min, sensor_max = self._load_cell.saturation_range() if safety_min <= sensor_min or safety_max >= sensor_max: cmd_err = self._printer.command_error raise cmd_err( @@ -508,7 +522,7 @@ def get_probe_drift_range(self, tare_counts, gcmd=None) -> Tuple[int, int]: drift_counts = int(counts_per_gram * drift_force) drift_min = int(tare_counts - drift_counts) drift_max = int(tare_counts + drift_counts) - sensor_min, sensor_max = self._load_cell.get_sensor().get_range() + sensor_min, sensor_max = self._load_cell.saturation_range() if drift_min <= sensor_min or drift_max >= sensor_max: cmd_err = self._printer.command_error raise cmd_err( @@ -559,6 +573,9 @@ def __init__( self._home_cmd = None self._query_cmd = None self._set_range_cmd = None + self._trigger_confirm_ticks = self._mcu.seconds_to_clock( + self._config_helper.get_trigger_confirm_time() + ) self._mcu.register_config_callback(self._build_config) self._printer.register_event_handler("klippy:connect", self._on_connect) @@ -585,7 +602,8 @@ def _build_config(self): ) self._home_cmd = self._mcu.lookup_command( "load_cell_probe_home oid=%c trsync_oid=%c trigger_reason=%c" - " error_reason=%c clock=%u rest_ticks=%u timeout=%u", + " error_reason=%c clock=%u rest_ticks=%u timeout=%u" + " trigger_confirm_ticks=%u", cq=self._cmd_queue, ) @@ -619,6 +637,9 @@ def set_endstop_range(self, gcmd: GCodeCommand = None): self._config_helper.get_grams_per_count(), ] self._set_range_cmd.send(args) + self._trigger_confirm_ticks = self._mcu.seconds_to_clock( + self._config_helper.get_trigger_confirm_time(gcmd) + ) self._sos_filter.reset_filter() def home_start(self, print_time): @@ -634,6 +655,7 @@ def home_start(self, print_time): clock, rest_ticks, self.WATCHDOG_MAX, + self._trigger_confirm_ticks, ], reqclock=clock, ) @@ -1017,9 +1039,7 @@ def get_position_endstop(self): return self._z_offset def get_status(self, eventtime): - status = self._tapping_move.get_status(eventtime) - status.update(self._tapping_move.get_status(eventtime)) - return status + return self._tapping_move.get_status(eventtime) class DriftFilterCalibration: @@ -1057,6 +1077,7 @@ def __init__( def calibrate(self, gcmd: GCodeCommand): try: + import numpy as np import scipy.signal as signal _ = signal @@ -1175,6 +1196,8 @@ def calibrate(self, gcmd: GCodeCommand): @staticmethod def _calculate_segment_slopes(force_data, sampling_rate, segment_duration): """Split the graph into segments and calculate a slope for each.""" + import numpy as np + segment_samples = int(segment_duration * sampling_rate) num_segments = len(force_data) // segment_samples segments = np.array_split(force_data, num_segments) @@ -1200,7 +1223,7 @@ def _apply_drift_filter(force_data, cutoff_freq, sampling_rate): def _run_calibration( self, - force_data: np.ndarray, + force_data, sampling_rate: float, segment_duration: float, slope_percentile: float, @@ -1210,6 +1233,8 @@ def _run_calibration( max_drift_rate: float, gcmd: GCodeCommand, ): + import numpy as np + current_cutoff = cutoff while current_cutoff <= max_cutoff_frequency: filtered_data = self._apply_drift_filter( @@ -1288,6 +1313,8 @@ def _finalize_callback(self, probe_offsets, results): pass def calibrate(self, gcmd: GCodeCommand): + import numpy as np + self._gcmd = gcmd gcmd.respond_info("Starting pullback_distance calibration...") bed_mesh: BedMesh = self._printer.lookup_object( diff --git a/src/load_cell_probe.c b/src/load_cell_probe.c index 5e48a4c5ed..d8daf28b34 100644 --- a/src/load_cell_probe.c +++ b/src/load_cell_probe.c @@ -37,8 +37,6 @@ typedef int64_t fixedQ48_t; #define ERROR_OVERFLOW 1 #define ERROR_WATCHDOG 2 #define ERROR_ADC_INVALID 3 -#define TRIGGER_CONFIRM_SAMPLES 2 - // Flags enum {FLAG_IS_HOMING = 1 << 0 , FLAG_IS_HOMING_TRIGGER = 1 << 1 @@ -48,12 +46,13 @@ enum {FLAG_IS_HOMING = 1 << 0 // Endstop Structure struct load_cell_probe { struct timer time; - uint32_t trigger_grams, trigger_ticks, last_sample_ticks, rest_ticks; + uint32_t trigger_grams, trigger_ticks, first_trigger_ticks; + uint32_t last_sample_ticks, rest_ticks, trigger_confirm_ticks; uint32_t homing_start_time; struct trsync *ts; int32_t safety_counts_min, safety_counts_max, tare_counts; uint8_t flags, trigger_reason, error_reason, watchdog_max - , watchdog_count, trigger_count; + , watchdog_count, trigger_count, sensor_ready; fixedQ16_t trigger_grams_fixed; fixedQ2_t grams_per_count; struct sos_filter *sf; @@ -103,61 +102,70 @@ clear_flag(uint8_t mask, struct load_cell_probe *lcp) lcp->flags &= ~mask; } -void +static void +trigger_terminal(struct load_cell_probe *lcp, uint8_t reason, uint32_t ticks) +{ + if (!is_flag_set(FLAG_IS_HOMING, lcp) + || is_flag_set(FLAG_IS_HOMING_TRIGGER, lcp)) + return; + lcp->trigger_ticks = ticks; + set_flag(FLAG_IS_HOMING_TRIGGER, lcp); + trsync_do_trigger(lcp->ts, reason); +} + +static void try_trigger(struct load_cell_probe *lcp, uint32_t ticks) { - uint8_t is_homing_triggered = is_flag_set(FLAG_IS_HOMING_TRIGGER, lcp); - if (!is_homing_triggered) { - // the first triggering sample when homing sets the trigger time - lcp->trigger_ticks = ticks; - // this flag latches until a reset, disabling further triggering - set_flag(FLAG_IS_HOMING_TRIGGER, lcp); - trsync_do_trigger(lcp->ts, lcp->trigger_reason); - } + trigger_terminal(lcp, lcp->trigger_reason, ticks); } -void -trigger_error(struct load_cell_probe *lcp, uint8_t error_code) +static void +trigger_error(struct load_cell_probe *lcp, uint8_t error_code, uint32_t ticks) +{ + trigger_terminal(lcp, lcp->error_reason + error_code, ticks); +} + +// Return true only after the scheduled homing start time. Samples and faults +// must use the same gate so an old bulk/capture event cannot abort a future +// move before that move has started. +static uint8_t +probe_is_active(struct load_cell_probe *lcp, uint32_t ticks) { - trsync_do_trigger(lcp->ts, lcp->error_reason + error_code); + if (!is_flag_set(FLAG_IS_HOMING, lcp) + || is_flag_set(FLAG_IS_HOMING_TRIGGER, lcp)) + return 0; + if (is_flag_set(FLAG_AWAIT_HOMING, lcp) + && timer_is_before(ticks, lcp->homing_start_time)) + return 0; + clear_flag(FLAG_AWAIT_HOMING, lcp); + return 1; } // Used by Sensors to report new raw ADC sample void -load_cell_probe_report_sample(struct load_cell_probe *lcp - , const int32_t sample) +load_cell_probe_report_sample_at(struct load_cell_probe *lcp + , const int32_t sample, uint32_t ticks) { - // only process samples when homing - uint8_t is_homing = is_flag_set(FLAG_IS_HOMING, lcp); - if (!is_homing) { + if (!probe_is_active(lcp, ticks)) return; - } // save new sample - uint32_t ticks = timer_read_time(); lcp->last_sample_ticks = ticks; lcp->watchdog_count = 0; - // do not trigger before homing start time - uint8_t await_homing = is_flag_set(FLAG_AWAIT_HOMING, lcp); - if (await_homing && timer_is_before(ticks, lcp->homing_start_time)) { - return; - } - clear_flag(FLAG_AWAIT_HOMING, lcp); - // check for safety limit violations const uint8_t is_safety_trigger = sample <= lcp->safety_counts_min || sample >= lcp->safety_counts_max; // too much force, this is an error while homing if (is_safety_trigger) { - trigger_error(lcp, ERROR_SAFETY_RANGE); + trigger_error(lcp, ERROR_SAFETY_RANGE, ticks); return; } // convert sample to grams const fixedQ48_t raw_grams = counts_to_grams(lcp, sample); if (overflows_int32(raw_grams)) { - trigger_error(lcp, ERROR_OVERFLOW); + trigger_error(lcp, ERROR_OVERFLOW, ticks); return; } @@ -172,19 +180,36 @@ load_cell_probe_report_sample(struct load_cell_probe *lcp lcp->trigger_count = 0; return; } - // Two consecutive qualified, filtered samples prevent a remaining - // one-frame impulse from ending a move. At 80 SPS this adds <=12.5ms. - if (lcp->trigger_count < TRIGGER_CONFIRM_SAMPLES) - lcp->trigger_count++; - if (lcp->trigger_count >= TRIGGER_CONFIRM_SAMPLES) - try_trigger(lcp, lcp->last_sample_ticks); + if (!lcp->trigger_count) { + lcp->trigger_count = 1; + lcp->first_trigger_ticks = ticks; + } + uint32_t confirm_at = lcp->first_trigger_ticks + + lcp->trigger_confirm_ticks; + if (!timer_is_before(ticks, confirm_at)) + // The first crossing is the best estimate of physical contact. The + // later samples only confirm that it was not a one-frame impulse. + try_trigger(lcp, lcp->first_trigger_ticks); +} + +void +load_cell_probe_report_sample(struct load_cell_probe *lcp, const int32_t sample) +{ + load_cell_probe_report_sample_at(lcp, sample, timer_read_time()); +} + +void +load_cell_probe_report_fault_at(struct load_cell_probe *lcp + , uint32_t sample_ticks) +{ + if (probe_is_active(lcp, sample_ticks)) + trigger_error(lcp, ERROR_ADC_INVALID, sample_ticks); } void -load_cell_probe_report_fault(struct load_cell_probe *lcp) +load_cell_probe_set_sensor_ready(struct load_cell_probe *lcp, uint8_t is_ready) { - if (is_flag_set(FLAG_IS_HOMING, lcp)) - trigger_error(lcp, ERROR_ADC_INVALID); + lcp->sensor_ready = !!is_ready; } // Timer callback that monitors for timeouts @@ -200,8 +225,13 @@ watchdog_event(struct timer *t) return SF_DONE; } + if (!lcp->sensor_ready) { + trigger_error(lcp, ERROR_ADC_INVALID, t->waketime); + return SF_DONE; + } if (lcp->watchdog_count > lcp->watchdog_max) { - trigger_error(lcp, ERROR_WATCHDOG); + trigger_error(lcp, ERROR_WATCHDOG, t->waketime); + return SF_DONE; } lcp->watchdog_count += 1; @@ -216,7 +246,7 @@ set_endstop_range(struct load_cell_probe *lcp , int32_t tare_counts, uint32_t trigger_grams , fixedQ2_t grams_per_count) { - if (!(safety_counts_max >= safety_counts_min)) { + if (safety_counts_max <= safety_counts_min) { shutdown("Safety range reversed"); } if (trigger_grams > MAX_TRIGGER_GRAMS) { @@ -224,7 +254,7 @@ set_endstop_range(struct load_cell_probe *lcp } // grams_per_count must be a positive fraction in Q2 format const fixedQ2_t one = 1UL << FIXEDQ2_FRAC_BITS; - if (grams_per_count < 0 || grams_per_count >= one) { + if (grams_per_count <= 0 || grams_per_count >= one) { shutdown("grams_per_count is invalid"); } lcp->safety_counts_min = safety_counts_min; @@ -246,8 +276,8 @@ command_config_load_cell_probe(uint32_t *args) lcp->watchdog_max = 0; lcp->watchdog_count = 0; lcp->trigger_count = 0; + lcp->sensor_ready = 1; lcp->sf = sos_filter_oid_lookup(args[1]); - set_endstop_range(lcp, 0, 0, 0, 0, 0); } DECL_COMMAND(command_config_load_cell_probe, "config_load_cell_probe" " oid=%c sos_filter_oid=%c"); @@ -294,6 +324,7 @@ command_load_cell_probe_home(uint32_t *args) lcp->homing_start_time = args[4]; lcp->rest_ticks = args[5]; lcp->watchdog_max = args[6]; + lcp->trigger_confirm_ticks = args[7]; lcp->watchdog_count = 0; lcp->trigger_count = 0; lcp->time.func = watchdog_event; @@ -303,7 +334,8 @@ command_load_cell_probe_home(uint32_t *args) } DECL_COMMAND(command_load_cell_probe_home, "load_cell_probe_home oid=%c trsync_oid=%c trigger_reason=%c" - " error_reason=%c clock=%u rest_ticks=%u timeout=%u"); + " error_reason=%c clock=%u rest_ticks=%u timeout=%u" + " trigger_confirm_ticks=%u"); void command_load_cell_probe_query_state(uint32_t *args) diff --git a/src/load_cell_probe.h b/src/load_cell_probe.h index 745fb29c80..a75888bcf2 100644 --- a/src/load_cell_probe.h +++ b/src/load_cell_probe.h @@ -6,6 +6,11 @@ struct load_cell_probe *load_cell_probe_oid_lookup(uint8_t oid); void load_cell_probe_report_sample(struct load_cell_probe *lce , int32_t sample); -void load_cell_probe_report_fault(struct load_cell_probe *lce); +void load_cell_probe_report_sample_at(struct load_cell_probe *lce + , int32_t sample, uint32_t sample_ticks); +void load_cell_probe_report_fault_at(struct load_cell_probe *lce + , uint32_t sample_ticks); +void load_cell_probe_set_sensor_ready(struct load_cell_probe *lce + , uint8_t is_ready); #endif // load_cell_probe.h diff --git a/src/sensor_hx711s.c b/src/sensor_hx711s.c index 1b155a02f8..e4cdf0ab74 100644 --- a/src/sensor_hx711s.c +++ b/src/sensor_hx711s.c @@ -12,7 +12,7 @@ #include "command.h" // DECL_COMMAND #include "sched.h" // sched_add_timer #include "sensor_bulk.h" // sensor_bulk_report -#include "load_cell_probe.h" // load_cell_probe_report_sample +#include "load_cell_probe.h" // load_cell_probe_report_sample_at #include #define MAX_SENSORS 4 @@ -24,8 +24,9 @@ #define QUALIFY_FRAMES 2 #define HX711S_OVERFLOW (1 << 1) -// A frame carries raw values even when it is invalid. The host must never -// feed a non-zero quality frame into a tare, filter, or probe decision. +// A frame carries raw values even when it is invalid. After checking and +// removing the wire-format tag, the host must never feed a non-zero quality +// frame into a tare, filter, or probe decision. #define HX711S_Q_SETTLING (1 << 0) #define HX711S_Q_NOT_READY (1 << 1) #define HX711S_Q_EXTRA_LOW (1 << 2) @@ -33,6 +34,12 @@ #define HX711S_Q_READ_OVERRUN (1 << 4) #define HX711S_Q_SATURATED (1 << 5) #define HX711S_Q_CHANNEL_SHIFT 8 +#define HX711S_Q_PROTOCOL_FAULT (HX711S_Q_NOT_READY | HX711S_Q_EXTRA_LOW \ + | HX711S_Q_POST_READ_LOW \ + | HX711S_Q_READ_OVERRUN) +#define HX711S_Q_HARD_FAULT (HX711S_Q_PROTOCOL_FAULT \ + | HX711S_Q_SATURATED) +#define HX711S_FRAME_TAG UINT32_C(0xa7110000) enum hx711s_state { HX711S_OFF, @@ -95,23 +102,33 @@ hx711s_set_clocks(struct hx711s_adc *h, uint8_t value) gpio_out_write(h->clks[i], value); } -// Read all devices in lockstep. SCK is high only while IRQs are masked; the -// low phase deliberately services pending IRQs before sampling stable DOUT. -static void +// Read all devices in lockstep. Sample DOUT after its rising-edge setup time +// while SCK is high, then check the final post-read level before servicing +// arbitrary pending IRQ work. This keeps every decision at a defined point +// in the data-sheet timing diagram while limiting each IRQ-off phase to 1us. +static uint8_t hx711s_raw_read(struct hx711s_adc *h, uint32_t *bits_out, uint8_t num_bits) { for (uint8_t i = 0; i < h->sensor_count; i++) bits_out[i] = 0; + uint8_t post_read_low = 0; while (num_bits--) { irq_disable(); hx711s_set_clocks(h, 1); hx711s_delay_noirq(); + for (uint8_t i = 0; i < h->sensor_count; i++) + bits_out[i] = (bits_out[i] << 1) | gpio_in_read(h->sdos[i]); hx711s_set_clocks(h, 0); + if (!num_bits) { + hx711s_delay_noirq(); + for (uint8_t i = 0; i < h->sensor_count; i++) + if (!gpio_in_read(h->sdos[i])) + post_read_low |= 1 << i; + } irq_enable(); hx711s_delay(); - for (uint8_t i = 0; i < h->sensor_count; i++) - bits_out[i] = (bits_out[i] << 1) | gpio_in_read(h->sdos[i]); } + return post_read_low; } static uint_fast8_t @@ -162,6 +179,8 @@ hx711s_begin_reset(struct hx711s_adc *h) // list, so remove it before changing its wake time and inserting it. sched_del_timer(&h->timer); h->pending_flag = 0; + if (h->lce) + load_cell_probe_set_sensor_ready(h->lce, 0); hx711s_set_clocks(h, 1); h->state = HX711S_RESET; h->settling_frames = SETTLING_FRAMES; @@ -174,36 +193,37 @@ static void hx711s_read_adc(struct hx711s_adc *h, uint8_t oid) { uint32_t adc[MAX_SENSORS], quality = 0; - int32_t counts[MAX_SENSORS]; + int32_t counts[MAX_SENSORS] = {0}; + uint32_t capture_ticks = timer_read_time(); uint8_t channel_mask = hx711s_ready_mask(h); uint8_t extras_mask = (1 << h->gain_channel) - 1; - if (channel_mask) + // Never clock an ADC that says its conversion is not ready. Doing so is + // outside the HX711 protocol and can turn a readiness fault into a second, + // misleading gain-bit fault. Preserve a diagnostic record and recover. + if (channel_mask) { quality |= HX711S_Q_NOT_READY; - hx711s_delay(); - hx711s_raw_read(h, adc, 24 + h->gain_channel); - hx711s_delay(); - - for (uint8_t i = 0; i < h->sensor_count; i++) { - uint32_t raw = adc[i] >> h->gain_channel; - if (raw & 0x800000) - raw |= 0xff000000; - counts[i] = raw; - if ((adc[i] & extras_mask) != extras_mask) { - quality |= HX711S_Q_EXTRA_LOW; - channel_mask |= 1 << i; - } - // After the final gain-selection pulse DOUT must remain high until - // the next conversion. A low line is a protocol/electrical fault, - // not a sample to silently hold or synthesize. - if (!gpio_in_read(h->sdos[i])) { - quality |= HX711S_Q_POST_READ_LOW; - channel_mask |= 1 << i; - } - if (counts[i] == INT32_C(0x007fffff) - || counts[i] == -INT32_C(0x00800000)) { - quality |= HX711S_Q_SATURATED; - channel_mask |= 1 << i; + } else { + uint8_t post_read_low = hx711s_raw_read( + h, adc, 24 + h->gain_channel); + for (uint8_t i = 0; i < h->sensor_count; i++) { + uint32_t raw = adc[i] >> h->gain_channel; + if (raw & 0x800000) + raw |= 0xff000000; + counts[i] = raw; + if ((adc[i] & extras_mask) != extras_mask) { + quality |= HX711S_Q_EXTRA_LOW; + channel_mask |= 1 << i; + } + if (post_read_low & (1 << i)) { + quality |= HX711S_Q_POST_READ_LOW; + channel_mask |= 1 << i; + } + if (counts[i] == INT32_C(0x007fffff) + || counts[i] == -INT32_C(0x00800000)) { + quality |= HX711S_Q_SATURATED; + channel_mask |= 1 << i; + } } } @@ -214,37 +234,45 @@ hx711s_read_adc(struct hx711s_adc *h, uint8_t oid) if (flags & HX711S_OVERFLOW) quality |= HX711S_Q_READ_OVERRUN; - if (h->state == HX711S_SETTLING || h->state == HX711S_QUALIFY) { + uint8_t was_qualifying = h->state == HX711S_SETTLING + || h->state == HX711S_QUALIFY; + if (was_qualifying) { if (!quality && h->state == HX711S_SETTLING && !--h->settling_frames) h->state = HX711S_QUALIFY; - else if (!quality && h->state == HX711S_QUALIFY && !--h->qualify_frames) + else if (!quality && h->state == HX711S_QUALIFY + && !--h->qualify_frames) { h->state = HX711S_ONLINE; + if (h->lce) + load_cell_probe_set_sensor_ready(h->lce, 1); + } quality |= HX711S_Q_SETTLING; } quality |= (uint32_t)channel_mask << HX711S_Q_CHANNEL_SHIFT; // A complete sample must fit before appending it. In the four-channel - // case each frame is 20 bytes, while the shared bulk buffer is 51 bytes: - // appending first when it already contains 40 bytes would overrun it. + // case each timestamped frame is 24 bytes, while the shared bulk buffer + // is 51 bytes: appending first when it contains 48 bytes would overrun it. if (h->sb.data_count + h->sample_bytes > ARRAY_SIZE(h->sb.data)) sensor_bulk_report(&h->sb, oid); + append_value(h, capture_ticks); for (uint8_t i = 0; i < h->sensor_count; i++) append_value(h, counts[i]); - append_value(h, quality); + // Tag the timestamped wire format. A host paired with an older MCU must + // fail closed instead of interpreting shifted count words as valid data. + append_value(h, HX711S_FRAME_TAG | quality); if (!quality && h->state == HX711S_ONLINE) { int32_t sum = 0; for (uint8_t i = 0; i < h->sensor_count; i++) sum += counts[i]; if (h->lce) - load_cell_probe_report_sample(h->lce, sum); - } else if (quality && h->lce) { - load_cell_probe_report_fault(h->lce); + load_cell_probe_report_sample_at(h->lce, sum, capture_ticks); + } else if ((quality & HX711S_Q_HARD_FAULT) && h->lce) { + load_cell_probe_report_fault_at(h->lce, capture_ticks); } - if (quality & (HX711S_Q_NOT_READY | HX711S_Q_EXTRA_LOW - | HX711S_Q_POST_READ_LOW | HX711S_Q_READ_OVERRUN)) + if (quality & HX711S_Q_PROTOCOL_FAULT) hx711s_begin_reset(h); } @@ -260,7 +288,12 @@ command_config_hx711s(uint32_t *args) shutdown("hx711s: gain_channel out of range 1-4"); h->sensor_count = args[1]; h->gain_channel = args[2]; - h->sample_bytes = BYTES_PER_VALUE * (h->sensor_count + 1); + // Each frame carries its MCU capture clock, all channel counts, and a + // quality word. The largest (four-channel) frame is 24 bytes, so exactly + // two frames fit in the shared 51-byte bulk buffer. + h->sample_bytes = BYTES_PER_VALUE * (h->sensor_count + 2); + if (h->sample_bytes > ARRAY_SIZE(h->sb.data)) + shutdown("hx711s: sample does not fit bulk buffer"); h->state = HX711S_OFF; } DECL_COMMAND(command_config_hx711s, @@ -283,6 +316,7 @@ hx711s_attach_load_cell_probe(uint32_t *args) { struct hx711s_adc *h = oid_lookup(args[0], command_config_hx711s); h->lce = load_cell_probe_oid_lookup(args[1]); + load_cell_probe_set_sensor_ready(h->lce, h->state == HX711S_ONLINE); } DECL_COMMAND(hx711s_attach_load_cell_probe, "hx711s_attach_load_cell_probe oid=%c load_cell_probe_oid=%c"); @@ -294,8 +328,13 @@ command_query_hx711s(uint32_t *args) h->rest_ticks = args[1]; if (!h->rest_ticks) { sched_del_timer(&h->timer); - hx711s_set_clocks(h, 1); + irq_disable(); h->state = HX711S_OFF; + h->pending_flag = 0; + irq_enable(); + hx711s_set_clocks(h, 1); + if (h->lce) + load_cell_probe_set_sensor_ready(h->lce, 0); return; } sensor_bulk_reset(&h->sb); @@ -321,7 +360,7 @@ hx711s_capture_task(void) uint8_t oid; struct hx711s_adc *h; foreach_oid(oid, h, command_config_hx711s) { - if (h->pending_flag) + if (h->state != HX711S_OFF && h->pending_flag) hx711s_read_adc(h, oid); } } diff --git a/src/sos_filter.c b/src/sos_filter.c index b2f416c2ea..a1a3b2d2b9 100644 --- a/src/sos_filter.c +++ b/src/sos_filter.c @@ -165,6 +165,8 @@ command_sos_filter_activate(uint32_t *args) shutdown("Filter section count larger than max_sections"); sf->n_sections = n_sections; const uint8_t coeff_int_bits = args[2]; + if (coeff_int_bits < 1 || coeff_int_bits > 30) + shutdown("Filter coefficient integer bits must be 1-30"); sf->coeff_frac_bits = (31 - coeff_int_bits); sf->coeff_rounding = (1 << (sf->coeff_frac_bits - 1)); // mark filter as ready to use diff --git a/test/test_hx711_reliability.py b/test/test_hx711_reliability.py new file mode 100644 index 0000000000..70b28faf68 --- /dev/null +++ b/test/test_hx711_reliability.py @@ -0,0 +1,204 @@ +import struct +from collections import Counter + +from klippy.extras.load_cell.hx711s import ( + FRAME_TAG, + Q_CHANNEL_SHIFT, + Q_EXTRA_LOW, + Q_FRAME_FORMAT, + Q_NOT_READY, + Q_SETTLING, + HX711SBase, + TimestampedBulkReader, + quality_flags, + quality_is_hard, +) +from klippy.extras.load_cell.load_cell import LoadCellSampleCollector + + +class FakeMcu: + def clock32_to_clock64(self, clock): + return clock + + def clock_to_print_time(self, clock): + return clock / 1000.0 + + +class FakeStatusCommand: + def __init__(self, overflows=0): + self.overflows = overflows + + def send(self, _args): + return {"possible_overflows": self.overflows, "next_sequence": 0} + + +class FakeBulkQueue: + def __init__(self, messages): + self.messages = messages + + def pull_queue(self): + messages, self.messages = self.messages, [] + return messages + + def clear_queue(self): + self.messages = [] + + +def make_hx711(sensor_count=2): + sensor = HX711SBase.__new__(HX711SBase) + sensor.sensor_count = sensor_count + sensor.mcu = FakeMcu() + sensor._health = Counter() + sensor._quality_totals = Counter() + sensor._channel_fault_totals = Counter() + sensor._last_hard_fault = None + return sensor + + +def test_quality_classification(): + assert not quality_is_hard(Q_SETTLING) + assert quality_is_hard(Q_SETTLING | Q_NOT_READY) + assert quality_is_hard(1 << Q_CHANNEL_SHIFT) + assert quality_flags(Q_SETTLING | Q_EXTRA_LOW) == ( + "settling", + "extra_low", + ) + + +def test_timestamped_conversion_keeps_fault_details_out_of_control_data(): + sensor = make_hx711() + samples = [ + (1000, 100, 200, FRAME_TAG | Q_SETTLING), + ( + 2000, + 110, + 210, + FRAME_TAG | Q_NOT_READY | (1 << (Q_CHANNEL_SHIFT + 1)), + ), + (3000, 120, 220, FRAME_TAG), + ] + + faults = sensor._convert_samples(samples) + + assert len(faults) == 2 + assert faults[0]["time"] == 1.0 + assert not faults[0]["hard"] + assert faults[1]["time"] == 2.0 + assert faults[1]["hard"] + assert faults[1]["channels"] == (1,) + assert faults[1]["flags"] == ("not_ready",) + assert samples[0][0] == 3.0 + assert samples[0][1] == 120 + assert samples[0][3] == 220 + assert sensor._health == { + "received": 3, + "fault": 2, + "settling": 1, + "hard": 1, + "valid": 1, + } + assert sensor._last_hard_fault == faults[1] + + +def test_untagged_wire_format_fails_closed(): + sensor = make_hx711() + samples = [(1000, 100, 200, 0)] + + faults = sensor._convert_samples(samples) + + assert samples == [] + assert len(faults) == 1 + assert faults[0]["hard"] + assert faults[0]["quality"] == Q_FRAME_FORMAT + assert faults[0]["flags"] == ("frame_format",) + + +def test_timestamped_reader_preserves_real_gaps_between_capture_clocks(): + reader = TimestampedBulkReader(FakeMcu(), " Date: Wed, 22 Jul 2026 22:02:39 -0400 Subject: [PATCH 05/11] probe: retry one invalid HX711 sample During carbon2u bed meshes, HX711S protocol faults such as not_ready, extra_low, and post_read_low correctly become an invalid-sample load-cell probe error. However, one torn HX711 frame during a probe point should retire that measurement and let the existing probe/mesh retry loop collect the point again instead of aborting the whole mesh. Handle only the explicit invalid HX711 sample diagnostic inside PrinterProbe._run_probe_with_retries(). Retract and retry the same probe point once, then allow any second invalid sample to propagate as the original hard error. Other load-cell failures, including drift safety, fixed-point overflow, watchdog timeout, and ordinary homing timeouts, still raise immediately. Add focused regression tests covering a successful one-shot retry, repeated invalid samples remaining fatal, and force safety errors not being retried. --- klippy/extras/probe.py | 16 +++- test/test_hx711_reliability.py | 139 +++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 1 deletion(-) diff --git a/klippy/extras/probe.py b/klippy/extras/probe.py index a9c5722d12..a275d55168 100644 --- a/klippy/extras/probe.py +++ b/klippy/extras/probe.py @@ -24,6 +24,8 @@ can travel further (the Z minimum position can be negative). """ +HX711_INVALID_SAMPLE_ERROR = "Load Cell Probe Error: invalid HX711 sample" + class RetryStrategy(IntEnum): # Probe strategy constants @@ -523,10 +525,22 @@ def _run_probe_with_retries( self, speed: float, retry_session: RetrySession, gcmd: GCodeCommand ) -> list[float]: """Probe for a single good result with retries based on strategy""" + hx711_invalid_retries = 0 while retry_session.can_retry(): self._move(retry_session.get_probe_position(), self.retry_speed) # Probe position - pos, is_good = self._probe(speed, gcmd) + try: + pos, is_good = self._probe(speed, gcmd) + except self.printer.command_error as e: + if ( + HX711_INVALID_SAMPLE_ERROR not in str(e) + or hx711_invalid_retries + ): + raise + hx711_invalid_retries += 1 + gcmd.respond_info("HX711 invalid sample detected. Retrying...") + self._retract(gcmd) + continue if retry_session.evaluate_probe(is_good): # return the x/y of the original requested location return list(retry_session.get_position() + (pos[2],)) diff --git a/test/test_hx711_reliability.py b/test/test_hx711_reliability.py index 70b28faf68..ebc9797ab6 100644 --- a/test/test_hx711_reliability.py +++ b/test/test_hx711_reliability.py @@ -14,6 +14,7 @@ quality_is_hard, ) from klippy.extras.load_cell.load_cell import LoadCellSampleCollector +from klippy.extras.probe import PrinterProbe class FakeMcu: @@ -202,3 +203,141 @@ def test_collector_preserves_legacy_batch_errors_without_fault_records(): _samples, errors = collector._finish_collecting() assert errors == (2, 1) + + +class ProbeCommandError(Exception): + pass + + +class FakeProbePrinter: + command_error = ProbeCommandError + + +class FakeGcmd: + def __init__(self): + self.messages = [] + + def respond_info(self, message): + self.messages.append(message) + + def error(self, message): + return ProbeCommandError(message) + + +class FakeProbeRetrySession: + def __init__(self): + self.positions = [(10.0, 20.0, None), (10.0, 20.0, None)] + self.evaluated = [] + + def can_retry(self): + return bool(self.positions) + + def get_probe_position(self): + return self.positions.pop(0) + + def evaluate_probe(self, is_good): + self.evaluated.append(is_good) + return is_good + + def get_position(self): + return (10.0, 20.0) + + def get_bad_probe_count(self): + return len(self.evaluated) + + def scrub_nozzle(self): + pass + + +def make_probe_runner(outcomes): + probe = PrinterProbe.__new__(PrinterProbe) + probe.printer = FakeProbePrinter() + probe.retry_speed = 7.0 + probe.moves = [] + probe.retracts = 0 + outcomes = list(outcomes) + + def move(pos, speed): + probe.moves.append((pos, speed)) + + def retract(_gcmd): + probe.retracts += 1 + + def do_probe(_speed, _gcmd): + outcome = outcomes.pop(0) + if isinstance(outcome, Exception): + raise outcome + return outcome + + probe._move = move + probe._retract = retract + probe._probe = do_probe + return probe + + +def test_probe_retries_one_invalid_hx711_sample(): + gcmd = FakeGcmd() + retry_session = FakeProbeRetrySession() + probe = make_probe_runner( + [ + ProbeCommandError( + "Load Cell Probe Error: invalid HX711 sample; " + "see sensor fault diagnostics" + ), + ([10.0, 20.0, -0.42], True), + ] + ) + + result = probe._run_probe_with_retries(5.0, retry_session, gcmd) + + assert result == [10.0, 20.0, -0.42] + assert probe.retracts == 1 + assert len(probe.moves) == 2 + assert retry_session.evaluated == [True] + assert gcmd.messages == ["HX711 invalid sample detected. Retrying..."] + + +def test_probe_does_not_retry_repeated_invalid_hx711_samples(): + gcmd = FakeGcmd() + retry_session = FakeProbeRetrySession() + probe = make_probe_runner( + [ + ProbeCommandError("Load Cell Probe Error: invalid HX711 sample"), + ProbeCommandError("Load Cell Probe Error: invalid HX711 sample"), + ] + ) + + try: + probe._run_probe_with_retries(5.0, retry_session, gcmd) + except ProbeCommandError: + pass + else: + assert False, "second invalid HX711 sample should remain a hard failure" + + assert probe.retracts == 1 + assert len(probe.moves) == 2 + assert retry_session.evaluated == [] + + +def test_probe_does_not_retry_hard_load_cell_safety_error(): + gcmd = FakeGcmd() + retry_session = FakeProbeRetrySession() + probe = make_probe_runner( + [ + ProbeCommandError( + "Load Cell Probe Error: force exceeded drift_safety_limit " + "before triggering!" + ) + ] + ) + + try: + probe._run_probe_with_retries(5.0, retry_session, gcmd) + except ProbeCommandError: + pass + else: + assert False, "safety failures must not be retried as transient samples" + + assert probe.retracts == 0 + assert len(probe.moves) == 1 + assert retry_session.evaluated == [] From 64593629b31ab1baa05d30db8474c3e7e5ddb508 Mon Sep 17 00:00:00 2001 From: Paul Swenson <1937248+pdscomp@users.noreply.github.com> Date: Wed, 22 Jul 2026 23:10:42 -0400 Subject: [PATCH 06/11] Retry transient HX711 collector faults during probing A single HX711 acquisition fault can surface through LoadCellSampleCollector validation as the generic "Sensor reported 1 acquisition errors and 0 bulk overflows while sampling" error. That happens during probe tare or tap sample validation before the existing PrinterProbe invalid-HX711 retry sees the MCU probe error string, so BED_MESH_CALIBRATE can abort on a transient recovery sample instead of retrying the probe.\n\nNormalize only HX711/hx711s probe-boundary collector failures with exactly one acquisition error and zero bulk overflows to the existing invalid HX711 sample probe error. The existing PrinterProbe retry limit then retries once and keeps repeated faults fatal. Bulk overflows, non-HX711 sensors, safety faults, and all generic LoadCell validation outside probing continue to fail normally.\n\nAdd regression coverage for collector acquisition retry, tare conversion, repeated acquisition faults, bulk overflow, and non-HX711 behavior. --- klippy/extras/load_cell/load_cell_probe.py | 34 ++++- test/test_hx711_reliability.py | 160 +++++++++++++++++++++ 2 files changed, 192 insertions(+), 2 deletions(-) diff --git a/klippy/extras/load_cell/load_cell_probe.py b/klippy/extras/load_cell/load_cell_probe.py index d2aaf143de..dd1d6f1009 100644 --- a/klippy/extras/load_cell/load_cell_probe.py +++ b/klippy/extras/load_cell/load_cell_probe.py @@ -12,7 +12,11 @@ from klippy.configfile import ConfigWrapper, PrinterConfig from klippy.extras.bed_mesh import BedMesh from klippy.extras.homing import PrinterHoming -from klippy.extras.probe import PrinterProbe, ProbePointsHelper +from klippy.extras.probe import ( + HX711_INVALID_SAMPLE_ERROR, + PrinterProbe, + ProbePointsHelper, +) from klippy.gcode import GCodeCommand, GCodeDispatch from klippy.printer import Printer from klippy.toolhead import ToolHead @@ -671,6 +675,10 @@ def clear_home(self): # Execute probing moves using the McuLoadCellProbe class LoadCellPrimitives: + SINGLE_ACQUISITION_ERROR = ( + "Sensor reported 1 acquisition errors and 0 bulk overflows while " + "sampling" + ) ERROR_MAP = { mcu.MCU_trsync.REASON_COMMS_TIMEOUT: "Communication timeout during " "homing", @@ -715,11 +723,31 @@ def _start_collector(self) -> LoadCellSampleCollector: collector.start_collecting(min_time=print_time) return collector + def _is_hx711_sensor(self) -> bool: + sensor_type = getattr(self._load_cell.sensor, "sensor_type", "") + return sensor_type in ("hx711", "hx711s") + + def _raise_invalid_hx711_sample(self): + raise self._printer.command_error( + f"{HX711_INVALID_SAMPLE_ERROR}; see sensor fault diagnostics" + ) + + def _raise_if_retryable_collector_error(self, error): + if ( + self._is_hx711_sensor() + and self.SINGLE_ACQUISITION_ERROR in str(error) + ): + self._raise_invalid_hx711_sample() + # pauses for the last move to complete and then # sets the endstop tare value and range def tare(self, gcmd=None): num_samples = self._config_helper.get_tare_samples(gcmd) - tare_counts_per_channel = self._load_cell.avg_counts(num_samples) + try: + tare_counts_per_channel = self._load_cell.avg_counts(num_samples) + except self._printer.command_error as e: + self._raise_if_retryable_collector_error(e) + raise self._load_cell.tare(tare_counts_per_channel) self._config_helper.assert_force_safety_limit(gcmd) # update sos_filter with any gcode parameter changes @@ -785,6 +813,8 @@ def probing_test(self, gcmd, timeout): return self.home_wait(print_time + timeout) def validate_samples(self, samples, errors): + if self._is_hx711_sensor() and errors == (1, 0): + self._raise_invalid_hx711_sample() self._load_cell.validate_samples(samples, errors) def get_status(self, eventtime): diff --git a/test/test_hx711_reliability.py b/test/test_hx711_reliability.py index ebc9797ab6..2e8b1c2236 100644 --- a/test/test_hx711_reliability.py +++ b/test/test_hx711_reliability.py @@ -14,6 +14,7 @@ quality_is_hard, ) from klippy.extras.load_cell.load_cell import LoadCellSampleCollector +from klippy.extras.load_cell.load_cell_probe import LoadCellPrimitives from klippy.extras.probe import PrinterProbe @@ -265,6 +266,8 @@ def retract(_gcmd): def do_probe(_speed, _gcmd): outcome = outcomes.pop(0) + if callable(outcome): + return outcome() if isinstance(outcome, Exception): raise outcome return outcome @@ -275,6 +278,72 @@ def do_probe(_speed, _gcmd): return probe +class FakePrimitiveSensor: + def __init__(self, sensor_type="hx711s"): + self.sensor_type = sensor_type + + +class FakePrimitiveLoadCell: + def __init__(self, sensor_type="hx711s"): + self.sensor = FakePrimitiveSensor(sensor_type) + self.validated = [] + self.tared = [] + + def validate_samples(self, samples, errors): + self.validated.append((samples, errors)) + if errors: + error_count, overflow_count = errors + raise ProbeCommandError( + "Sensor reported %i acquisition errors and %i bulk " + "overflows while sampling" % (error_count, overflow_count) + ) + + def avg_counts(self, _num_samples): + raise ProbeCommandError( + "Sensor reported 1 acquisition errors and 0 bulk overflows while " + "sampling" + ) + + def tare(self, counts): + self.tared.append(counts) + + +class FakeProbeConfigHelper: + def get_tare_samples(self, _gcmd): + return 4 + + def assert_force_safety_limit(self, _gcmd): + pass + + +class FakeContinuousTareFilterHelper: + def update_from_command(self, _gcmd): + pass + + +class FakeMcuLoadCellProbe: + def set_endstop_range(self, _gcmd): + pass + + +def make_load_cell_primitives(sensor_type="hx711s"): + primitives = LoadCellPrimitives.__new__(LoadCellPrimitives) + primitives._printer = FakeProbePrinter() + primitives._load_cell = FakePrimitiveLoadCell(sensor_type) + primitives._config_helper = FakeProbeConfigHelper() + primitives._continuous_tare_filter_helper = FakeContinuousTareFilterHelper() + primitives._mcu_load_cell_probe = FakeMcuLoadCellProbe() + return primitives + + +def collector_error_probe(primitives, errors): + def do_probe(): + primitives.validate_samples([], errors) + return [10.0, 20.0, -0.42], True + + return do_probe + + def test_probe_retries_one_invalid_hx711_sample(): gcmd = FakeGcmd() retry_session = FakeProbeRetrySession() @@ -297,6 +366,38 @@ def test_probe_retries_one_invalid_hx711_sample(): assert gcmd.messages == ["HX711 invalid sample detected. Retrying..."] +def test_probe_retries_one_hx711_collector_acquisition_error(): + gcmd = FakeGcmd() + retry_session = FakeProbeRetrySession() + primitives = make_load_cell_primitives() + probe = make_probe_runner( + [ + collector_error_probe(primitives, (1, 0)), + ([10.0, 20.0, -0.42], True), + ] + ) + + result = probe._run_probe_with_retries(5.0, retry_session, gcmd) + + assert result == [10.0, 20.0, -0.42] + assert probe.retracts == 1 + assert len(probe.moves) == 2 + assert retry_session.evaluated == [True] + assert primitives._load_cell.validated == [] + assert gcmd.messages == ["HX711 invalid sample detected. Retrying..."] + + +def test_probe_tare_converts_one_hx711_collector_acquisition_error(): + primitives = make_load_cell_primitives() + + try: + primitives.tare(FakeGcmd()) + except ProbeCommandError as e: + assert "Load Cell Probe Error: invalid HX711 sample" in str(e) + else: + assert False, "single HX711 tare acquisition error should be retryable" + + def test_probe_does_not_retry_repeated_invalid_hx711_samples(): gcmd = FakeGcmd() retry_session = FakeProbeRetrySession() @@ -319,6 +420,65 @@ def test_probe_does_not_retry_repeated_invalid_hx711_samples(): assert retry_session.evaluated == [] +def test_probe_does_not_retry_repeated_hx711_collector_acquisition_errors(): + gcmd = FakeGcmd() + retry_session = FakeProbeRetrySession() + primitives = make_load_cell_primitives() + probe = make_probe_runner( + [ + collector_error_probe(primitives, (1, 0)), + collector_error_probe(primitives, (1, 0)), + ] + ) + + try: + probe._run_probe_with_retries(5.0, retry_session, gcmd) + except ProbeCommandError: + pass + else: + assert False, "second collector acquisition error should remain fatal" + + assert probe.retracts == 1 + assert len(probe.moves) == 2 + assert retry_session.evaluated == [] + + +def test_probe_does_not_retry_collector_bulk_overflow(): + gcmd = FakeGcmd() + retry_session = FakeProbeRetrySession() + primitives = make_load_cell_primitives() + probe = make_probe_runner([collector_error_probe(primitives, (1, 1))]) + + try: + probe._run_probe_with_retries(5.0, retry_session, gcmd) + except ProbeCommandError as e: + assert "1 bulk overflows" in str(e) + else: + assert False, "bulk overflow should remain a hard failure" + + assert probe.retracts == 0 + assert len(probe.moves) == 1 + assert retry_session.evaluated == [] + + +def test_probe_does_not_retry_non_hx711_collector_acquisition_error(): + gcmd = FakeGcmd() + retry_session = FakeProbeRetrySession() + primitives = make_load_cell_primitives("ads1220") + probe = make_probe_runner([collector_error_probe(primitives, (1, 0))]) + + try: + probe._run_probe_with_retries(5.0, retry_session, gcmd) + except ProbeCommandError as e: + assert "1 acquisition errors" in str(e) + else: + assert False, "non-HX711 acquisition errors should remain generic" + + assert probe.retracts == 0 + assert len(probe.moves) == 1 + assert retry_session.evaluated == [] + + def test_probe_does_not_retry_hard_load_cell_safety_error(): gcmd = FakeGcmd() retry_session = FakeProbeRetrySession() From 13952564f42e95a39b526e44b05b2f46e8fb7e29 Mon Sep 17 00:00:00 2001 From: Paul Swenson <1937248+pdscomp@users.noreply.github.com> Date: Thu, 23 Jul 2026 01:00:13 -0400 Subject: [PATCH 07/11] hx711s: reclassify timing misses as non-fatal sample skips A poll or read racing the free-running HX711 conversion clock produces a NOT_READY frame (window closed before the read) or an EXTRA_LOW|POST_READ_LOW frame (a conversion completed mid-read). At 80 SPS on the CC1 bed this happens about once per 12 s, and each event forced a full power-down reset, a 7-frame recovery, and a probe-aborting hard fault. No bed mesh could survive the steady-state fault rate even with a one-retry budget at two layers. Treat the three timing flags as timing misses instead of protocol faults: the frame remains excluded from tare, filter, and probe decisions, but the MCU neither resets nor reports a probe fault, and the host counts it in a new "timing" health bucket instead of failing the collection window. READ_OVERRUN and SATURATED keep the existing fault and reset semantics. A stuck ADC still fails closed: 40 consecutive timing misses (~0.5 s at 80 SPS) tag the frame with the new Q_TIMING_STREAK bit, which the host classifies as a hard fault, and the MCU reports a probe fault and resets. Old hosts see the unknown bit and fail closed as well. Validation: 15 host tests (timing classification, escalation marking, collector behavior), ruff clean, STM32F401 build passes. --- klippy/extras/load_cell/hx711s.py | 32 ++++++++---- src/sensor_hx711s.c | 53 ++++++++++++++++---- test/test_hx711_reliability.py | 83 ++++++++++++++++++++++++++----- 3 files changed, 134 insertions(+), 34 deletions(-) diff --git a/klippy/extras/load_cell/hx711s.py b/klippy/extras/load_cell/hx711s.py index b8d72faa66..7d4e49e2fc 100644 --- a/klippy/extras/load_cell/hx711s.py +++ b/klippy/extras/load_cell/hx711s.py @@ -29,8 +29,10 @@ Q_READ_OVERRUN = 1 << 4 Q_SATURATED = 1 << 5 Q_FRAME_FORMAT = 1 << 6 +Q_TIMING_STREAK = 1 << 7 Q_CHANNEL_SHIFT = 8 Q_CHANNEL_MASK = 0xF << Q_CHANNEL_SHIFT +Q_TIMING_MISS = Q_NOT_READY | Q_EXTRA_LOW | Q_POST_READ_LOW FRAME_TAG = 0xA7110000 FRAME_TAG_MASK = 0xFFFF0000 QUALITY_FLAGS = ( @@ -41,14 +43,16 @@ (Q_READ_OVERRUN, "read_overrun"), (Q_SATURATED, "saturated"), (Q_FRAME_FORMAT, "frame_format"), + (Q_TIMING_STREAK, "timing_streak"), ) def quality_is_hard(quality: int) -> bool: - # Channel bits and unknown future bits are conservatively hard. The only - # non-control frame that is expected during successful recovery is a pure - # settling/qualification frame. - return bool(quality & ~Q_SETTLING) + # Settling/qualification frames and timing-miss frames (a read racing the + # free-running conversion clock) are excluded from control data but are + # not faults; the MCU escalates a persistent timing streak itself. + # Channel bits only locate a frame's fault and carry no severity. + return bool(quality & ~(Q_SETTLING | Q_TIMING_MISS | Q_CHANNEL_MASK)) def quality_flags(quality: int) -> tuple[str, ...]: @@ -160,6 +164,7 @@ def __init__( self._pending_log_channels = Counter() self._pending_log_hard = 0 self._pending_log_settling = 0 + self._pending_log_timing = 0 self._last_fault_log = float("-inf") ppins = self.printer.lookup_object("pins") @@ -304,13 +309,17 @@ def _convert_samples(self, samples): } faults.append(fault) self._health["fault"] += 1 - self._health["hard" if hard else "settling"] += 1 + if hard: + self._health["hard"] += 1 + self._last_hard_fault = fault + elif quality & Q_TIMING_MISS: + self._health["timing"] += 1 + else: + self._health["settling"] += 1 for flag in flags: self._quality_totals[flag] += 1 for channel in channels: self._channel_fault_totals[channel] += 1 - if hard: - self._last_hard_fault = fault continue self._health["valid"] += 1 converted = [round(ptime, 6)] @@ -323,8 +332,6 @@ def _convert_samples(self, samples): return faults def _log_faults(self, eventtime, faults): - if not any(fault["hard"] for fault in faults): - return for fault in faults: for flag in fault["flags"]: self._pending_log_flags[flag] += 1 @@ -332,6 +339,8 @@ def _log_faults(self, eventtime, faults): self._pending_log_channels[channel] += 1 if fault["hard"]: self._pending_log_hard += 1 + elif fault["quality"] & Q_TIMING_MISS: + self._pending_log_timing += 1 else: self._pending_log_settling += 1 if not self._pending_log_hard or eventtime < self._last_fault_log + 1.0: @@ -348,9 +357,11 @@ def _log_faults(self, eventtime, faults): or "none" ) logging.warning( - "%s: HX711 faults: hard=%d recovery=%d; flags=[%s]; channels=[%s]", + "%s: HX711 faults: hard=%d timing=%d recovery=%d;" + " flags=[%s]; channels=[%s]", self.name, self._pending_log_hard, + self._pending_log_timing, self._pending_log_settling, flag_summary, channel_summary, @@ -358,6 +369,7 @@ def _log_faults(self, eventtime, faults): self._pending_log_flags.clear() self._pending_log_channels.clear() self._pending_log_hard = self._pending_log_settling = 0 + self._pending_log_timing = 0 self._last_fault_log = eventtime def _start_measurements(self): diff --git a/src/sensor_hx711s.c b/src/sensor_hx711s.c index e4cdf0ab74..45079b4203 100644 --- a/src/sensor_hx711s.c +++ b/src/sensor_hx711s.c @@ -34,11 +34,17 @@ #define HX711S_Q_READ_OVERRUN (1 << 4) #define HX711S_Q_SATURATED (1 << 5) #define HX711S_Q_CHANNEL_SHIFT 8 -#define HX711S_Q_PROTOCOL_FAULT (HX711S_Q_NOT_READY | HX711S_Q_EXTRA_LOW \ - | HX711S_Q_POST_READ_LOW \ - | HX711S_Q_READ_OVERRUN) -#define HX711S_Q_HARD_FAULT (HX711S_Q_PROTOCOL_FAULT \ - | HX711S_Q_SATURATED) +// Timing misses are the polling/read racing the free-running conversion +// clock: the frame is invalid and stays out of control data, but the ADC is +// healthy and the next poll resynchronizes. Only a persistent streak (a +// genuinely stuck ADC) escalates to the fault and reset path. +#define HX711S_Q_TIMING_MISS (HX711S_Q_NOT_READY | HX711S_Q_EXTRA_LOW \ + | HX711S_Q_POST_READ_LOW) +#define HX711S_Q_GENUINE_FAULT (HX711S_Q_READ_OVERRUN | HX711S_Q_SATURATED) +// Set on a timing-miss frame when a persistent streak proves a stuck ADC. +// Bit 6 is reserved for the host-synthetic frame-format flag. +#define HX711S_Q_TIMING_STREAK (1 << 7) +#define HX711S_TIMING_STREAK_LIMIT 40 #define HX711S_FRAME_TAG UINT32_C(0xa7110000) enum hx711s_state { @@ -54,7 +60,7 @@ struct hx711s_adc { struct timer timer; uint32_t rest_ticks; uint8_t pending_flag, sensor_count, gain_channel, sample_bytes; - uint8_t state, settling_frames, qualify_frames; + uint8_t state, settling_frames, qualify_frames, timing_streak; struct gpio_in sdos[MAX_SENSORS]; struct gpio_out clks[MAX_SENSORS]; struct sensor_bulk sb; @@ -185,6 +191,7 @@ hx711s_begin_reset(struct hx711s_adc *h) h->state = HX711S_RESET; h->settling_frames = SETTLING_FRAMES; h->qualify_frames = QUALIFY_FRAMES; + h->timing_streak = 0; h->timer.waketime = timer_read_time() + timer_from_us(POWERDOWN_US); sched_add_timer(&h->timer); } @@ -249,6 +256,14 @@ hx711s_read_adc(struct hx711s_adc *h, uint8_t oid) } quality |= (uint32_t)channel_mask << HX711S_Q_CHANNEL_SHIFT; + // A persistent timing streak means a genuinely stuck ADC: tag the frame + // hard in-band so the host records it as a fault, then reset below. + uint8_t escalate = (quality & HX711S_Q_TIMING_MISS) + && !(quality & HX711S_Q_GENUINE_FAULT) + && h->timing_streak + 1 >= HX711S_TIMING_STREAK_LIMIT; + if (escalate) + quality |= HX711S_Q_TIMING_STREAK; + // A complete sample must fit before appending it. In the four-channel // case each timestamped frame is 24 bytes, while the shared bulk buffer // is 51 bytes: appending first when it contains 48 bytes would overrun it. @@ -262,18 +277,34 @@ hx711s_read_adc(struct hx711s_adc *h, uint8_t oid) // fail closed instead of interpreting shifted count words as valid data. append_value(h, HX711S_FRAME_TAG | quality); + if (quality & HX711S_Q_GENUINE_FAULT) { + h->timing_streak = 0; + if (h->lce) + load_cell_probe_report_fault_at(h->lce, capture_ticks); + if (quality & HX711S_Q_READ_OVERRUN) + hx711s_begin_reset(h); + return; + } + if (quality & HX711S_Q_TIMING_MISS) { + if (escalate) { + // Persistent timing failure: the ADC is genuinely stuck. + h->timing_streak = 0; + if (h->lce) + load_cell_probe_report_fault_at(h->lce, capture_ticks); + hx711s_begin_reset(h); + } else { + h->timing_streak++; + } + return; + } + h->timing_streak = 0; if (!quality && h->state == HX711S_ONLINE) { int32_t sum = 0; for (uint8_t i = 0; i < h->sensor_count; i++) sum += counts[i]; if (h->lce) load_cell_probe_report_sample_at(h->lce, sum, capture_ticks); - } else if ((quality & HX711S_Q_HARD_FAULT) && h->lce) { - load_cell_probe_report_fault_at(h->lce, capture_ticks); } - - if (quality & HX711S_Q_PROTOCOL_FAULT) - hx711s_begin_reset(h); } void diff --git a/test/test_hx711_reliability.py b/test/test_hx711_reliability.py index 2e8b1c2236..afa28fe9e5 100644 --- a/test/test_hx711_reliability.py +++ b/test/test_hx711_reliability.py @@ -7,7 +7,11 @@ Q_EXTRA_LOW, Q_FRAME_FORMAT, Q_NOT_READY, + Q_POST_READ_LOW, + Q_READ_OVERRUN, + Q_SATURATED, Q_SETTLING, + Q_TIMING_STREAK, HX711SBase, TimestampedBulkReader, quality_flags, @@ -59,24 +63,34 @@ def make_hx711(sensor_count=2): def test_quality_classification(): assert not quality_is_hard(Q_SETTLING) - assert quality_is_hard(Q_SETTLING | Q_NOT_READY) - assert quality_is_hard(1 << Q_CHANNEL_SHIFT) + # Timing misses are excluded from control data but are not faults. + assert not quality_is_hard(Q_SETTLING | Q_NOT_READY) + assert not quality_is_hard(Q_NOT_READY | Q_EXTRA_LOW | Q_POST_READ_LOW) + # Channel bits only locate a fault and carry no severity. + assert not quality_is_hard(1 << Q_CHANNEL_SHIFT) + # Genuine faults stay hard. + assert quality_is_hard(Q_READ_OVERRUN) + assert quality_is_hard(Q_SATURATED) + assert quality_is_hard(Q_FRAME_FORMAT) + assert quality_is_hard(Q_NOT_READY | Q_SATURATED) + # An MCU-escalated timing streak is a hard fault. + assert quality_is_hard(Q_NOT_READY | Q_TIMING_STREAK) + assert quality_is_hard(1 << 12) assert quality_flags(Q_SETTLING | Q_EXTRA_LOW) == ( "settling", "extra_low", ) + assert quality_flags(Q_NOT_READY | Q_TIMING_STREAK) == ( + "not_ready", + "timing_streak", + ) def test_timestamped_conversion_keeps_fault_details_out_of_control_data(): sensor = make_hx711() samples = [ (1000, 100, 200, FRAME_TAG | Q_SETTLING), - ( - 2000, - 110, - 210, - FRAME_TAG | Q_NOT_READY | (1 << (Q_CHANNEL_SHIFT + 1)), - ), + (2000, 110, 210, FRAME_TAG | Q_READ_OVERRUN), (3000, 120, 220, FRAME_TAG), ] @@ -87,8 +101,8 @@ def test_timestamped_conversion_keeps_fault_details_out_of_control_data(): assert not faults[0]["hard"] assert faults[1]["time"] == 2.0 assert faults[1]["hard"] - assert faults[1]["channels"] == (1,) - assert faults[1]["flags"] == ("not_ready",) + assert faults[1]["channels"] == () + assert faults[1]["flags"] == ("read_overrun",) assert samples[0][0] == 3.0 assert samples[0][1] == 120 assert samples[0][3] == 220 @@ -102,6 +116,48 @@ def test_timestamped_conversion_keeps_fault_details_out_of_control_data(): assert sensor._last_hard_fault == faults[1] +def test_timing_miss_is_not_a_fault(): + sensor = make_hx711() + samples = [ + ( + 1000, + 100, + 200, + FRAME_TAG | Q_NOT_READY | (1 << (Q_CHANNEL_SHIFT + 1)), + ), + ( + 2000, + 110, + 210, + FRAME_TAG + | Q_EXTRA_LOW + | Q_POST_READ_LOW + | (1 << Q_CHANNEL_SHIFT), + ), + (3000, 120, 220, FRAME_TAG), + ] + + faults = sensor._convert_samples(samples) + + assert len(faults) == 2 + assert not faults[0]["hard"] + assert faults[0]["channels"] == (1,) + assert faults[0]["flags"] == ("not_ready",) + assert not faults[1]["hard"] + assert faults[1]["channels"] == (0,) + assert faults[1]["flags"] == ("extra_low", "post_read_low") + # Timing frames stay out of control data; only the clean frame remains. + assert len(samples) == 1 + assert samples[0][0] == 3.0 + assert sensor._health == { + "received": 3, + "fault": 2, + "timing": 2, + "valid": 1, + } + assert sensor._last_hard_fault is None + + def test_untagged_wire_format_fails_closed(): sensor = make_hx711() samples = [(1000, 100, 200, 0)] @@ -156,12 +212,13 @@ def add_client(self, callback): def fault(time, hard): + quality = Q_READ_OVERRUN if hard else Q_SETTLING return { "time": time, "counts": (0, 0), - "quality": Q_NOT_READY if hard else Q_SETTLING, - "wire_quality": FRAME_TAG | (Q_NOT_READY if hard else Q_SETTLING), - "flags": ("not_ready",) if hard else ("settling",), + "quality": quality, + "wire_quality": FRAME_TAG | quality, + "flags": ("read_overrun",) if hard else ("settling",), "channels": (), "hard": hard, } From 426ac30bc9e6a05ddd77abfa19be664df779b6c2 Mon Sep 17 00:00:00 2001 From: Paul Swenson <1937248+pdscomp@users.noreply.github.com> Date: Thu, 23 Jul 2026 08:00:12 -0400 Subject: [PATCH 08/11] load_cell_probe: let an armed trigger confirm finish across the drift band The drift-safety range was checked before trigger evaluation on every sample. With the 12.5 ms confirm, the trigger needs two or more consecutive above-threshold samples at 80 SPS, so a fast genuine contact ramp (cleaning-pass probes, stiff edge contact) crossed the raw drift band in one or two samples and the homing move died as ERROR_SAFETY_RANGE before the confirm could complete. Upstream triggers on the first filtered crossing and never sees this race; the confirm window is a rewrite addition for impulse rejection. Evaluate the drift band only when no confirm is armed: the first sample of a contact streak must still be inside the band (a lost tare or gross spike still errors), but once armed the confirm owns the outcome and may complete across the band. Below the trigger threshold the band still applies and any armed confirm resets. Reproduced on CC1: hot LOADCELL_Z_HOME failed intermittently with "force exceeded drift_safety_limit before triggering!" while 2 mm/s PROBE passed; cleaning-pass contact peaks reach ~475 g against the +/-1000 g band. STM32F401 build passes; host-side tests unaffected (15 passed, ruff clean). --- src/load_cell_probe.c | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src/load_cell_probe.c b/src/load_cell_probe.c index d8daf28b34..658164c612 100644 --- a/src/load_cell_probe.c +++ b/src/load_cell_probe.c @@ -156,11 +156,6 @@ load_cell_probe_report_sample_at(struct load_cell_probe *lcp // check for safety limit violations const uint8_t is_safety_trigger = sample <= lcp->safety_counts_min || sample >= lcp->safety_counts_max; - // too much force, this is an error while homing - if (is_safety_trigger) { - trigger_error(lcp, ERROR_SAFETY_RANGE, ticks); - return; - } // convert sample to grams const fixedQ48_t raw_grams = counts_to_grams(lcp, sample); @@ -176,20 +171,34 @@ load_cell_probe_report_sample_at(struct load_cell_probe *lcp int64_t magnitude = filtered_grams; if (magnitude < 0) magnitude = -magnitude; - if (magnitude < lcp->trigger_grams_fixed) { - lcp->trigger_count = 0; + if (magnitude >= lcp->trigger_grams_fixed) { + // Above threshold: physical contact in progress. The first sample of + // a contact streak must still pass the safety gate, so a lost tare + // or a gross single-sample spike errors out. Once a confirm is + // armed it owns the outcome: a fast genuine ramp crosses the drift + // band within a sample or two, and must be allowed to finish the + // confirm window instead of dying as a safety error first. + if (!lcp->trigger_count) { + if (is_safety_trigger) { + trigger_error(lcp, ERROR_SAFETY_RANGE, ticks); + return; + } + lcp->trigger_count = 1; + lcp->first_trigger_ticks = ticks; + } + const uint32_t confirm_at = lcp->first_trigger_ticks + + lcp->trigger_confirm_ticks; + if (!timer_is_before(ticks, confirm_at)) + // The first crossing is the best estimate of physical contact. + // The later samples only confirm it was not a one-frame impulse. + try_trigger(lcp, lcp->first_trigger_ticks); return; } - if (!lcp->trigger_count) { - lcp->trigger_count = 1; - lcp->first_trigger_ticks = ticks; - } - uint32_t confirm_at = lcp->first_trigger_ticks - + lcp->trigger_confirm_ticks; - if (!timer_is_before(ticks, confirm_at)) - // The first crossing is the best estimate of physical contact. The - // later samples only confirm that it was not a one-frame impulse. - try_trigger(lcp, lcp->first_trigger_ticks); + // Below threshold: an armed confirm dies here, and the drift band still + // applies while no contact is being confirmed. + lcp->trigger_count = 0; + if (is_safety_trigger) + trigger_error(lcp, ERROR_SAFETY_RANGE, ticks); } void From a4ceadd374656a54fc222fb10b356063200ee86a Mon Sep 17 00:00:00 2001 From: Paul Swenson <1937248+pdscomp@users.noreply.github.com> Date: Thu, 23 Jul 2026 08:07:39 -0400 Subject: [PATCH 09/11] load_cell_probe: default to first-crossing triggering (confirm_time=0) Every reference implementation (upstream Kalico, garethky's community driver, the OpenCentauri fork, and Prusa's loadcell) triggers on the first filtered threshold crossing; the 12.5 ms confirmation window was an Elegoo-stock-ism carried into the rewrite. At 80 SPS the window is one sample period, so the trigger needed two or more above-threshold samples, and the extra post-contact travel pushed stiff-contact ramps across the raw drift band before the confirm could complete. With confirm_time defaulting to 0 the MCU triggers on the first within-band filtered crossing, exactly matching upstream semantics; the arming-gate restructure then degenerates to upstream's ordering. The parameter remains for configs that want impulse rejection, and the first crossing is always the reported contact time. --- docs/Config_Reference.md | 7 ++++--- klippy/extras/load_cell/load_cell_probe.py | 12 +++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/docs/Config_Reference.md b/docs/Config_Reference.md index f29f5747cf..c75ee87830 100644 --- a/docs/Config_Reference.md +++ b/docs/Config_Reference.md @@ -6141,11 +6141,12 @@ sensor_type: # The default is +/-1Kg. #trigger_force: 75.0 # The force that the probe will trigger at. 75g is the default. -#trigger_confirm_time: 0.0125 +#trigger_confirm_time: 0.0 # The minimum time, in seconds, that force must remain above trigger_force # before a probe is accepted. The first threshold crossing remains the -# reported contact time. The default is 0.0125 seconds. Set to 0 to disable -# confirmation. +# reported contact time. The default is 0: trigger on the first filtered +# crossing, matching upstream behavior. Set a positive value to confirm +# over a fixed duration and reject one-frame impulses. #drift_filter_cutoff_frequency: 0.8 # Enable optional continuous taring while homing & probing to reject drift. # The value is a frequency, in Hz, below which drift will be ignored. This diff --git a/klippy/extras/load_cell/load_cell_probe.py b/klippy/extras/load_cell/load_cell_probe.py index dd1d6f1009..1168fb2372 100644 --- a/klippy/extras/load_cell/load_cell_probe.py +++ b/klippy/extras/load_cell/load_cell_probe.py @@ -409,14 +409,16 @@ def __init__( self._trigger_force_param = intParamHelper( config, "trigger_force", default=75, minval=10, maxval=250 ) - # Require force to remain above the threshold for a fixed duration, - # independent of the ADC's configured sample rate. The MCU reports - # the first crossing as contact time after the later samples confirm - # it was not a one-frame impulse. + # Optional confirmation window: force may remain above the threshold + # for a fixed duration before triggering. The default of 0 triggers + # on the first filtered crossing, matching upstream Kalico; a + # positive value rejects one-frame impulses at the cost of extra + # post-contact travel. The first crossing is always the reported + # contact time. self._trigger_confirm_time_param = floatParamHelper( config, "trigger_confirm_time", - default=0.0125, + default=0.0, minval=0.0, maxval=0.100, ) From 87f1c4f710242f9df4cb8d1b8b410d5b5f3f8efb Mon Sep 17 00:00:00 2001 From: Paul Swenson <1937248+pdscomp@users.noreply.github.com> Date: Thu, 23 Jul 2026 08:57:18 -0400 Subject: [PATCH 10/11] hx711s: reject single-sample channel glitches before the probe path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The r14 mesh campaign failed again with "force exceeded drift_safety_limit before triggering!" — with first-crossing triggering that requires the first above-threshold sample to be beyond the tare +/-1000 g band, i.e. a single-sample jump of >1000 g on the sum. A real 2 mm/s contact cannot do that in 25 um of travel; these are one-frame electrical glitches (ch3 dominant, matching both the old Elegoo 100k-count heuristic and the spike filter on OpenCentauri/kalico hx711s-new). Port the hx711s-new per-channel spike filter: a sample whose channel delta vs the last good value exceeds 100k counts is held back from the probe (the raw frame still streams to the host for diagnostics). A second consecutive sample near the suspicious value confirms a genuine force step and is forwarded; an immediate return to the last good value confirms an isolated glitch. State re-seeds after a power-cycle reset. Genuine fast ramps are delayed by at most one sample (~12 ms), one-frame spikes can no longer trip the drift band. --- src/sensor_hx711s.c | 80 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 4 deletions(-) diff --git a/src/sensor_hx711s.c b/src/sensor_hx711s.c index 45079b4203..f353e1395a 100644 --- a/src/sensor_hx711s.c +++ b/src/sensor_hx711s.c @@ -45,6 +45,12 @@ // Bit 6 is reserved for the host-synthetic frame-format flag. #define HX711S_Q_TIMING_STREAK (1 << 7) #define HX711S_TIMING_STREAK_LIMIT 40 + +// A genuine load change moves each channel by well under this between +// samples (a 2 mm/s approach at 80 SPS moves the four-channel sum by only +// ~5k counts), so a larger single-sample jump on one channel is a glitch, +// not a load change. +#define HX711S_SPIKE_THRESHOLD 100000 #define HX711S_FRAME_TAG UINT32_C(0xa7110000) enum hx711s_state { @@ -61,6 +67,8 @@ struct hx711s_adc { uint32_t rest_ticks; uint8_t pending_flag, sensor_count, gain_channel, sample_bytes; uint8_t state, settling_frames, qualify_frames, timing_streak; + int32_t last_good_counts[MAX_SENSORS], pending_counts[MAX_SENSORS]; + uint8_t have_last_counts, have_pending_counts; struct gpio_in sdos[MAX_SENSORS]; struct gpio_out clks[MAX_SENSORS]; struct sensor_bulk sb; @@ -192,10 +200,22 @@ hx711s_begin_reset(struct hx711s_adc *h) h->settling_frames = SETTLING_FRAMES; h->qualify_frames = QUALIFY_FRAMES; h->timing_streak = 0; + h->have_last_counts = 0; + h->have_pending_counts = 0; h->timer.waketime = timer_read_time() + timer_from_us(POWERDOWN_US); sched_add_timer(&h->timer); } +static void +hx711s_report_sum(struct hx711s_adc *h, int32_t *counts, + uint32_t capture_ticks) +{ + int32_t sum = 0; + for (uint8_t i = 0; i < h->sensor_count; i++) + sum += counts[i]; + load_cell_probe_report_sample_at(h->lce, sum, capture_ticks); +} + static void hx711s_read_adc(struct hx711s_adc *h, uint8_t oid) { @@ -299,11 +319,63 @@ hx711s_read_adc(struct hx711s_adc *h, uint8_t oid) } h->timing_streak = 0; if (!quality && h->state == HX711S_ONLINE) { - int32_t sum = 0; + if (!h->lce) + return; + // Reject a single-sample glitch on any channel before forwarding the + // summed sample to the probe. The raw frame above still streams to + // the host, so the bad channel stays visible in diagnostics. A + // second sample near the suspicious value confirms a genuine force + // step; an immediate return to the last good value confirms an + // isolated glitch. This keeps a one-frame corrupt read from tripping + // the drift safety range without filtering a real collision forever. + // (Adapted from OpenCentauri/kalico hx711s-new.) + if (!h->have_last_counts) { + for (uint8_t i = 0; i < h->sensor_count; i++) + h->last_good_counts[i] = counts[i]; + h->have_last_counts = 1; + hx711s_report_sum(h, counts, capture_ticks); + return; + } + uint8_t bad_mask = 0; + for (uint8_t i = 0; i < h->sensor_count; i++) { + int32_t delta = counts[i] - h->last_good_counts[i]; + if (delta > HX711S_SPIKE_THRESHOLD + || delta < -HX711S_SPIKE_THRESHOLD) + bad_mask |= 1 << i; + } + if (!bad_mask) { + // A valid sample confirms any pending sample was a glitch. + h->have_pending_counts = 0; + for (uint8_t i = 0; i < h->sensor_count; i++) + h->last_good_counts[i] = counts[i]; + hx711s_report_sum(h, counts, capture_ticks); + return; + } + if (h->have_pending_counts) { + uint8_t pending_match = 1; + for (uint8_t i = 0; i < h->sensor_count; i++) { + int32_t delta = counts[i] - h->pending_counts[i]; + if (delta > HX711S_SPIKE_THRESHOLD + || delta < -HX711S_SPIKE_THRESHOLD) { + pending_match = 0; + break; + } + } + if (pending_match) { + // Two consecutive samples agree on the new value: a genuine + // force step that must not be filtered forever. + h->have_pending_counts = 0; + for (uint8_t i = 0; i < h->sensor_count; i++) + h->last_good_counts[i] = counts[i]; + hx711s_report_sum(h, counts, capture_ticks); + return; + } + } + // Store (or replace) the suspicious sample and wait for the next + // one to judge it. for (uint8_t i = 0; i < h->sensor_count; i++) - sum += counts[i]; - if (h->lce) - load_cell_probe_report_sample_at(h->lce, sum, capture_ticks); + h->pending_counts[i] = counts[i]; + h->have_pending_counts = 1; } } From c368f84ec516f5d639aeddcb99d2c967957ab8e7 Mon Sep 17 00:00:00 2001 From: Paul Swenson <1937248+pdscomp@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:55:57 -0400 Subject: [PATCH 11/11] load_cell: tolerate rare bulk overflows on timestamped sensors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The round-5 mesh campaign reached 5 consecutive passes before attempt 6 died to "Sensor reported 0 acquisition errors and 1 bulk overflows while sampling" — a single lost bulk message after ~45 minutes of streaming, almost certainly a transient host scheduling hiccup. Failing a whole 7.5-minute macro for one lost frame is far too strict: hx711s frames are individually timestamped, so a lost message is a known gap rather than silent corruption, and the consumers are gap-tolerant by design (tare uses a median over many frames; tap data is shape-validated by TapAnalysis; the MCU trigger path does not use bulk data at all). Add a max_tolerated_overflows sensor capability (2 for hx711s, 0 default so other sensors keep the old strict behavior) and honor it in LoadCell.validate_samples: (0, <=2) logs a warning and continues, larger or mixed losses stay fatal. Covers both the tare (avg_counts) and tap (TappingMove) collection paths. --- klippy/extras/load_cell/hx711s.py | 7 +++++ klippy/extras/load_cell/load_cell.py | 19 +++++++++--- test/test_hx711_reliability.py | 45 ++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/klippy/extras/load_cell/hx711s.py b/klippy/extras/load_cell/hx711s.py index 7d4e49e2fc..9241703b44 100644 --- a/klippy/extras/load_cell/hx711s.py +++ b/klippy/extras/load_cell/hx711s.py @@ -142,6 +142,13 @@ def pull_samples(self): class HX711SBase(LoadCellSensor): + # This sensor's frames are individually timestamped, so a rare lost bulk + # message is a known gap rather than silent corruption: tare uses a + # median over many frames and tap data is shape-validated downstream. + # LoadCell.validate_samples tolerates up to this many bulk overflows + # per session; larger losses still fail the command. + max_tolerated_overflows = 2 + def __init__( self, config, diff --git a/klippy/extras/load_cell/load_cell.py b/klippy/extras/load_cell/load_cell.py index d08fb9be54..86f755d7c1 100644 --- a/klippy/extras/load_cell/load_cell.py +++ b/klippy/extras/load_cell/load_cell.py @@ -833,10 +833,21 @@ def avg_counts(self, num_samples: int | None = None) -> tuple[int, ...]: def validate_samples(self, samples, errors): if errors: error_count, overflow_count = errors - raise self.printer.command_error( - "Sensor reported %i acquisition errors and %i bulk " - "overflows while sampling" % (error_count, overflow_count) - ) + # A rare lost bulk message is survivable for sensors whose + # frames are individually timestamped: the gap is known, not + # silent corruption. Anything larger stays fatal. + tolerated = getattr(self.sensor, "max_tolerated_overflows", 0) + if error_count == 0 and overflow_count <= tolerated: + logging.warning( + "load_cell: tolerating %i bulk overflow(s) this " + "sampling session", + overflow_count, + ) + else: + raise self.printer.command_error( + "Sensor reported %i acquisition errors and %i bulk " + "overflows while sampling" % (error_count, overflow_count) + ) # check individual channels for saturated readings range_min, range_max = self.channel_saturation_range() first_channel_col = SampleStructure.channel_counts_col(0) diff --git a/test/test_hx711_reliability.py b/test/test_hx711_reliability.py index afa28fe9e5..b8d207afbf 100644 --- a/test/test_hx711_reliability.py +++ b/test/test_hx711_reliability.py @@ -338,6 +338,9 @@ def do_probe(_speed, _gcmd): class FakePrimitiveSensor: def __init__(self, sensor_type="hx711s"): self.sensor_type = sensor_type + self.max_tolerated_overflows = ( + 2 if sensor_type == "hx711s" else 0 + ) class FakePrimitiveLoadCell: @@ -350,6 +353,14 @@ def validate_samples(self, samples, errors): self.validated.append((samples, errors)) if errors: error_count, overflow_count = errors + if ( + error_count == 0 + and overflow_count <= self.sensor.max_tolerated_overflows + ): + # Mirrors LoadCell.validate_samples: a small number of bulk + # overflows on a timestamped sensor is a known gap, not + # silent corruption, and is tolerated without a retry. + return raise ProbeCommandError( "Sensor reported %i acquisition errors and %i bulk " "overflows while sampling" % (error_count, overflow_count) @@ -518,6 +529,40 @@ def test_probe_does_not_retry_collector_bulk_overflow(): assert retry_session.evaluated == [] +def test_probe_tolerates_single_collector_bulk_overflow(): + gcmd = FakeGcmd() + retry_session = FakeProbeRetrySession() + primitives = make_load_cell_primitives() + probe = make_probe_runner([collector_error_probe(primitives, (0, 1))]) + + probe._run_probe_with_retries(5.0, retry_session, gcmd) + + # A lone bulk overflow on the timestamped hx711 stream is a known gap, + # not corruption: the probe succeeds without spending a retry. + assert primitives._load_cell.validated == [([], (0, 1))] + assert probe.retracts == 0 + assert len(probe.moves) == 1 + assert retry_session.evaluated == [True] + + +def test_probe_does_not_tolerate_large_collector_bulk_overflow(): + gcmd = FakeGcmd() + retry_session = FakeProbeRetrySession() + primitives = make_load_cell_primitives() + probe = make_probe_runner([collector_error_probe(primitives, (0, 3))]) + + try: + probe._run_probe_with_retries(5.0, retry_session, gcmd) + except ProbeCommandError as e: + assert "3 bulk overflows" in str(e) + else: + assert False, "losing 3 frames in one session should stay fatal" + + assert probe.retracts == 0 + assert len(probe.moves) == 1 + assert retry_session.evaluated == [] + + def test_probe_does_not_retry_non_hx711_collector_acquisition_error(): gcmd = FakeGcmd() retry_session = FakeProbeRetrySession()