Conversation
bus.py's i2c_transfer() calls invoke_shutdown() on any non-SUCCESS I2C status, which crashes the printer on transient NACK errors from the SGP40. This is too aggressive for a non-critical VOC sensor. Changes: - Add _patch_i2c() to replace invoke_shutdown() with command_error on this device's MCU_I2C instance, scoped only to the SGP40 (not other devices) - Add _read() retry loop: up to 5 attempts with 500ms delay between each, catching command_error — mirrors the SHT3x pattern from Klipper PR #6975 and aligns with the exception type established in Klipper PR #7206 - Wrap _handle_step() in try/except: on failure, log the error, reset measuring state, and reschedule 5 intervals later rather than stopping - Use retry=False on i2c_read() to prevent MCU-level serial retries from triggering a second NACK on an already-consumed SGP40 read buffer - Rename _read(len=) to _read(count=) to avoid shadowing the builtin
Extend the invoke_shutdown -> command_error patch to any ref sensor that exposes an i2c attribute. This prevents a BME280 configured as ref_temp_sensor or ref_humidity_sensor from triggering a printer shutdown on a transient I2C NACK, since it shares the same bus and the same glitch susceptibility as the SGP40. Move the patch call into _check_ref_sensor, which already holds the sensor object, and generalize _patch_i2c() to accept any MCU_I2C instance.
The I2C error handling on ref_temp_sensor and ref_humidity_sensor is modified to raise instead of invoke_shutdown on NACK errors. Document that this makes them unsafe to use for heater control.
Kalico's MCU_I2C has no i2c_transfer_cmd attribute and its i2c_read() takes no retry parameter, but it also doesn't call invoke_shutdown() on I2C errors so the patch is a no-op there anyway. - Use getattr() guard in _patch_i2c() to handle missing i2c_transfer_cmd - Drop retry=False from i2c_read() call; with _patch_i2c() in place it was no longer load-bearing for correctness on Klipper, and it broke Kalico's narrower i2c_read() signature - Restore Kalico minimum version in README
Patching i2c_transfer as an instance method proved unreliable — the original bus.py method was still being called in practice. Replacing i2c_transfer_cmd (a plain data attribute) sidesteps Python's method descriptor resolution entirely, so command_error is raised before invoke_shutdown is ever reached. Kalico support is removed: its MCU firmware calls shutdown() directly on any I2C NACK (i2ccmds.c i2c_shutdown_on_err), which cannot be intercepted at the Python level.
If the same I2C sensor is configured as both ref_temp_sensor and ref_humidity_sensor, _patch_i2c was called twice on the same i2c object, wrapping _SafeTransferCmd inside another _SafeTransferCmd. Moving the class to module level allows isinstance() to detect an already-patched object and return early.
The retry loop called reactor.pause() up to 5 times with 500ms waits, potentially stalling the reactor dispatch loop for 2.5 seconds. The except-Exception handler in _handle_step already provides retry semantics by rescheduling at 5x sampling_interval on any failure, so the in-function retries were redundant.
The except block covered the entire step — including i2c_write, temperature and humidity sensor lookups — not just the read. "Error reading data" was misleading when the failure came from elsewhere.
The measurement loop tolerates transient NACKs via _handle_step's exception handler, which could mislead a reader into thinking _init_sgp40 should be similarly lenient. The comment clarifies that startup failure is deliberate: a sensor that won't respond at connect time is a real error.
The safety constraint was only visible inside the config comment block. A user skimming the README would miss it entirely. The admonition places it prominently in the Configuration section where it will be seen.
The calibration state assignment, reference sensor lookups, and cmd build are all in-memory and cannot raise command_error. Keeping them inside the try block meant bugs in that code would be caught and logged as I2C errors rather than surfacing cleanly. Moving them before the try leaves only the SGP40 read and write inside the guarded block.
|
Hi @thetic I like the new approach. One thing I've observed doing this the quick and dirty way (just commenting out the invoke_shutdown command inside of bus.py) is that it works perfectly for SGP40 with your code loop as it is, but BMEx80 appears to have some special handling. The BMEx80 code from what I can tell treats any failure as a "oops, fail gracefully and stop updating until restart" event. We may need to re-init the BME280 when we hit this event otherwise the temp/humidity compensation gets locked to whatever it was when the BME280 fails. From my testing, it seems like most of the I2C NACKs are benign (both START_NACK, START_READ_NACK, etc), but a NACK on SGP40 can sometimes cause a bus timeout on BME280. Relevant test over on klipper discourse I'll test this out when I can, probably not till Thursday though |
BME280 permanently stops its sample timer when it catches an I2C error. Track ref sensors with sample timers and reschedule them each cycle while their reported temp is 0, allowing recovery without a printer restart.
|
I've been running this for about two weeks now without a nuisance restart and without seeing either the sgp40s or bme280s stop responding. my current uptime is about a week. |
|
Hey @thetic that's great to hear! I haven't gotten to test but will flash today and see. One question about the ref sensor code - I've seen when the BME280 gets stuck (after SGP40 failure causes an I2C Bus timeout) it's temp doesn't go to 0, it locks to whatever it was at time of failure (i.e. it the print will be over and back to 25C and the bme280 reports 56C. Your code appears to only look at 0 value sensors so does it catch this fail condition? |
|
The The "locked at last reading" behavior you observed is almost certainly specific to commenting out With the |
* origin/main: Add cross-sensor variance floor synchronization via sync_with
|
Quick update: I can now confirm the BME280 recovery works in practice. After the error bursts I see in my logs, successive queries to the BME280s return different values, so they're not locked to a stale reading. The |
2 participants
Summary
invoke_shutdown()calls from Klipper'sbus.py:i2c_transfer()by replacingi2c_transfer_cmdwith a wrapper that raisescommand_errorinstead on non-SUCCESS I2C status_handle_stepin a try/except; on failure, logs the error and backs off for 5× the sampling interval rather than crashing the printerRequirements
MCU firmware must be compiled and flashed from Klipper v0.13.0-159 or newer. A stale MCU firmware build may not support the
i2c_transfercommand that this patch relies on, and the NACK interception will not take effect.Test plan
ref_temp_sensorandref_humidity_sensorpoint to the same sensor (double-patch guard)