Printer Model: Any
MCU / Printerboard: Raspberry Pi 4 (doesn’t matter actually)
Host → Raspberry Pi 4 klippy.log (16.7 KB)
I think commit d120a31 (docs: Note 'config' object shouldn't be accessed after initial load · Klipper3d/klipper@d120a31 · GitHub) accidentally broke the ADS1X1X module.
All “self.config = config” lines were removed from any module that stored a copy of “config”, but in this case, “config” actually isn’t the global “config” object. Looking at the code (-> klippy/extras/ads1x1x.py) it is rather a “pinconfig” represented as an integer value. It is needed for sampling the pins.
An easy solution to this would be to revert the latest change and rename this specifig “config” to pinconfig, so that it does not get messed up with the config object.
If you agree on this, I’d try to create a PR on this (if I manage to do so).
I think you are correct. FWIW, I would recommend changing the name of the attribute to config_regor the like to eliminate ambiguity between the pin’s register value and the module configuration.
Thanks. I agree that I introduced a bug in that commit. I can revert that change to ads1x1x.py, however it would be nice to choose a different variable name to avoid future confusion like that.
Does something like the following work?
--- a/klippy/extras/ads1x1x.py
+++ b/klippy/extras/ads1x1x.py
@@ -210,28 +210,28 @@ class ADS1X1X_chip:
raise pins.error('ADS1x1x pin %s is not valid' % \
pin_params['pin'])
- config = 0
- config |= (ADS1X1X_OS['OS_SINGLE'] & \
- ADS1X1X_REG_CONFIG['OS_MASK'])
- config |= (ADS1X1X_MUX[pin_params['pin']] & \
- ADS1X1X_REG_CONFIG['MULTIPLEXER_MASK'])
- config |= (self.pga & ADS1X1X_REG_CONFIG['PGA_MASK'])
+ pcfg = 0
+ pcfg |= (ADS1X1X_OS['OS_SINGLE'] & \
+ ADS1X1X_REG_CONFIG['OS_MASK'])
+ pcfg |= (ADS1X1X_MUX[pin_params['pin']] & \
+ ADS1X1X_REG_CONFIG['MULTIPLEXER_MASK'])
+ pcfg |= (self.pga & ADS1X1X_REG_CONFIG['PGA_MASK'])
# Have to use single mode, because in continuous, it never reaches
# idle state, which we use to determine if the sampling is done.
- config |= (ADS1X1X_MODE['single'] & \
+ pcfg |= (ADS1X1X_MODE['single'] & \
ADS1X1X_REG_CONFIG['MODE_MASK'])
# lowest sample rate per default, until report time has been set in
# setup_adc_sample
- config |= (self.comp_mode \
- & ADS1X1X_REG_CONFIG['COMPARATOR_MODE_MASK'])
- config |= (self.comp_polarity \
- & ADS1X1X_REG_CONFIG['COMPARATOR_POLARITY_MASK'])
- config |= (self.comp_latching \
- & ADS1X1X_REG_CONFIG['COMPARATOR_LATCHING_MASK'])
- config |= (self.comp_queue \
- & ADS1X1X_REG_CONFIG['COMPARATOR_QUEUE_MASK'])
-
- pin_obj = ADS1X1X_pin(self, config)
+ pcfg |= (self.comp_mode \
+ & ADS1X1X_REG_CONFIG['COMPARATOR_MODE_MASK'])
+ pcfg |= (self.comp_polarity \
+ & ADS1X1X_REG_CONFIG['COMPARATOR_POLARITY_MASK'])
+ pcfg |= (self.comp_latching \
+ & ADS1X1X_REG_CONFIG['COMPARATOR_LATCHING_MASK'])
+ pcfg |= (self.comp_queue \
+ & ADS1X1X_REG_CONFIG['COMPARATOR_QUEUE_MASK'])
+
+ pin_obj = ADS1X1X_pin(self, pcfg)
if pin in self._pins:
raise pins.error(
'pin %s for chip %s is used multiple times' \
@@ -250,8 +250,8 @@ class ADS1X1X_chip:
logging.exception("ADS1X1X: error while resetting device")
def is_ready(self):
- config = self._read_register(ADS1X1X_REG_POINTER['CONFIG'])
- return bool((config & ADS1X1X_REG_CONFIG['OS_MASK']) == \
+ cfg = self._read_register(ADS1X1X_REG_POINTER['CONFIG'])
+ return bool((cfg & ADS1X1X_REG_CONFIG['OS_MASK']) == \
ADS1X1X_OS['OS_IDLE'])
def calculate_sample_rate(self):
@@ -281,7 +281,7 @@ class ADS1X1X_chip:
(sample_rate, sample_rate_bits) = self.calculate_sample_rate()
for pin in self._pins.values():
- pin.config = (pin.config & ~ADS1X1X_REG_CONFIG['DATA_RATE_MASK']) \
+ pin.pcfg = (pin.pcfg & ~ADS1X1X_REG_CONFIG['DATA_RATE_MASK']) \
| (sample_rate_bits & ADS1X1X_REG_CONFIG['DATA_RATE_MASK'])
self.delay = 1 / float(sample_rate)
@@ -289,7 +289,7 @@ class ADS1X1X_chip:
def sample(self, pin):
with self._mutex:
try:
- self._write_register(ADS1X1X_REG_POINTER['CONFIG'], pin.config)
+ self._write_register(ADS1X1X_REG_POINTER['CONFIG'], pin.pcfg)
self._reactor.pause(self._reactor.monotonic() + self.delay)
start_time = self._reactor.monotonic()
while not self.is_ready():
@@ -318,9 +318,10 @@ class ADS1X1X_chip:
self._i2c.i2c_write(data)
class ADS1X1X_pin:
- def __init__(self, chip, config):
+ def __init__(self, chip, pcfg):
self.mcu = chip.mcu
self.chip = chip
+ self.pcfg = pcfg
self.invalid_count = 0