mirror of
https://github.com/Klipper3d/klipper.git
synced 2026-10-19 01:59:55 -05:00
sensor_cs1237: don't sample the 27th clock as data
The 27th SCLK pulse of a CS1237 conversion frame does not carry conversion data - per the datasheet it forces DOUT high until the next conversion is ready. Its sampled level therefore depends on when the chip drives the line rather than on the conversion result. Sampling it as a data bit and requiring it to read back as 1 produces spurious SERIAL_DATA_DESYNCHRONISED errors on MCUs under load, where the bit-banged read can be preempted between clocks. On a toolhead MCU running the CS1237 at 640sps this yields roughly 30 errors per second, which makes load_cell_probe unusable - every probe aborts with "Load cell sensor reported errors while probing". Still clock all 27 pulses, but discard the last one and base the desync check on the reserved bit instead. The update1 bit used for config verification is preserved. This is the approach used by the out-of-tree QIDI Q2 driver in MisterSheikh/Qidi_Q2_Mainline_Klipper, where the 27th clock has been issued without sampling since f9f6cf0 (Feb 2026) and the reserved-bit desync check was added in 1ee516d (Aug 2026). That driver ran reliably on the same hardware for months, which is what identified the 27th bit as the difference once the in-tree driver started reporting desyncs. Co-authored-by: Ahmed Sheikh <ahmed.ali.sheikh1998@gmail.com> Signed-off-by: Ahmed Sheikh <ahmed.ali.sheikh1998@gmail.com> Signed-off-by: Koshik Oleg <qwertyboy58@gmail.com>
This commit is contained in:
1 parent
738441794e
commit
eefd197ac5
1 file changed
+9
-6
+9
-6
@@ -189,8 +189,11 @@ cs1237_read_adc(struct cs1237_adc *cs, uint8_t oid)
|
||||
raw = cs1237_read_bits(cs, 24);
|
||||
cs1237_write_config(cs);
|
||||
} else {
|
||||
// Bits 25-27 are update1, reserved zero, and forced high.
|
||||
raw = cs1237_read_bits(cs, 27);
|
||||
// Bits 25-26 are update1 and a reserved zero. Clock 27 only
|
||||
// forces DOUT high until the next conversion is ready - its
|
||||
// level depends on when the chip drives the line, not on the
|
||||
// conversion, so discard it instead of checking it.
|
||||
raw = cs1237_read_bits(cs, 27) >> 1;
|
||||
}
|
||||
|
||||
irq_disable();
|
||||
@@ -205,15 +208,15 @@ cs1237_read_adc(struct cs1237_adc *cs, uint8_t oid)
|
||||
if (old_flags & CS_CONFIG_PENDING)
|
||||
return;
|
||||
|
||||
uint_fast8_t status = raw & 0x07;
|
||||
uint32_t counts = raw >> 3;
|
||||
uint_fast8_t status = raw & 0x03;
|
||||
uint32_t counts = raw >> 2;
|
||||
if (counts & 0x800000)
|
||||
counts |= 0xff000000;
|
||||
|
||||
uint_fast8_t error = 0;
|
||||
if ((status & 0x03) != 0x01)
|
||||
if (status & 0x01)
|
||||
error = SE_DESYNC;
|
||||
else if ((old_flags & CS_CONFIG_VERIFY) && !(status & 0x04))
|
||||
else if ((old_flags & CS_CONFIG_VERIFY) && !(status & 0x02))
|
||||
error = SE_CONFIG;
|
||||
else if (old_flags & CS_OVERFLOW)
|
||||
error = SE_READ_TOO_LONG;
|
||||
|
||||
Reference in new issue
Block a user