Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 40 additions & 4 deletions klippy/extras/hx71x.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@ def __init__(
default_sample_rate,
gain_options,
default_gain,
sps_bits,
):
self.printer = printer = config.get_printer()
self.name = config.get_name().split()[-1]
self.last_error_count = 0
self.consecutive_fails = 0
self.sensor_type = sensor_type
self.consecutive_fails = 0
# Chip options
dout_pin_name = config.get("dout_pin")
sclk_pin_name = config.get("sclk_pin")
Expand All @@ -50,6 +51,17 @@ def __init__(
self.sps = config.getchoice(
"sample_rate", sample_rate_options, default=default_sample_rate
)
# HX71708 configures the sample rate instead of the gain/channel.
# We forward the sps bits to hx71x_read_adc so that it can read the extra bits required
if sensor_type == "hx71708":
self.gain_or_sps = 1
self.sps_bits = config.getchoice(
Copy link
Copy Markdown
Contributor

@garethky garethky Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like this is being tacked on the side. If there are going to be different configurable properties for different sensor types then we should accept that 100%. It looks like the sub types should not have that responsibility.

Some unique config logic is needed for each sensor type and then the resulting extra_bits get passed in to this class.

For the hx71708 specifically, the gain doesn't need to be read, since its not configurable.

I could argue something similar about the HX711, its SPS cant be set in software. I've already had people complain that they set the SPS but it didn't work. Maybe I need to have HX711_10 and HX711_80 as sensor types so people don't get the wrong idea.

"sample_rate", sps_bits, default=default_sample_rate
)
else:
self.gain_or_sps = 0
self.sps_bits = 0

# gain/channel choices
self.gain_channel = int(
config.getchoice("gain", gain_options, default=default_gain)
Expand All @@ -70,8 +82,15 @@ def __init__(
# Command Configuration
self.query_hx71x_cmd = None
mcu.add_config_cmd(
"config_hx71x oid=%d gain_channel=%d dout_pin=%s sclk_pin=%s"
% (self.oid, self.gain_channel, self.dout_pin, self.sclk_pin)
"config_hx71x oid=%d gain_or_sps=%d sps_bits=%d gain_channel=%d dout_pin=%s sclk_pin=%s"
% (
self.oid,
self.gain_or_sps,
self.sps_bits,
self.gain_channel,
self.dout_pin,
self.sclk_pin,
)
)
mcu.add_config_cmd(
"query_hx71x oid=%d rest_ticks=0" % (self.oid,), on_restart=True
Expand Down Expand Up @@ -179,6 +198,7 @@ def __init__(self, config):
# HX711 gain/channel options
{"A-128": 1, "B-32": 2, "A-64": 3},
"A-128",
{},
)


Expand All @@ -193,7 +213,23 @@ def __init__(self, config):
# HX717 gain/channel options
{"A-128": 1, "B-64": 2, "A-64": 3, "B-8": 4},
"A-128",
{},
)


class HX71708(HX71xBase):
def __init__(self, config):
super(HX71708, self).__init__(
config,
"hx71708",
# HX717 sps options
{320: 320, 80: 80, 20: 20, 10: 10},
320,
# HX717 gain/channel options
{"A-128": 1},
"A-128",
{320: 4, 3: 80, 20: 2, 10: 1},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like the 80 SPS option is transposed

{320: 4, 3: 80, 20: 2, 10: 1},
{320: 4, 80: 3, 20: 2, 10: 1},

)


HX71X_SENSOR_TYPES = {"hx711": HX711, "hx717": HX717}
HX71X_SENSOR_TYPES = {"hx711": HX711, "hx717": HX717, "hx71708": HX71708}
37 changes: 26 additions & 11 deletions src/sensor_hx71x.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

struct hx71x_adc {
struct timer timer;
uint8_t gain_or_sps;
uint8_t sps_bits;
uint8_t gain_channel; // the gain+channel selection (1-4)
uint8_t flags;
uint32_t rest_ticks;
Expand Down Expand Up @@ -77,7 +79,7 @@ hx71x_delay(void)

// Read 'num_bits' from the sensor
static uint32_t
hx71x_raw_read(struct gpio_in dout, struct gpio_out sclk, int num_bits)
hx71x_raw_read(struct gpio_in dout, struct gpio_out sclk, uint_fast8_t num_bits)
{
uint32_t bits_read = 0;
while (num_bits--) {
Expand Down Expand Up @@ -146,8 +148,15 @@ static void
hx71x_read_adc(struct hx71x_adc *hx71x, uint8_t oid)
{
// Read from sensor
uint_fast8_t gain_channel = hx71x->gain_channel;
uint32_t adc = hx71x_raw_read(hx71x->dout, hx71x->sclk, 24 + gain_channel);
uint_fast8_t gain_or_sps = hx71x->gain_or_sps;

uint_fast8_t extra_bits;
if (gain_or_sps == 1) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just have a single argument called "extra_bits" or maybe "setting_bits" and used that throughtout. That gets rid of the gain_or_sps flag and sps_bits

Even the error check is checking the same range so it would still function correctly.

extra_bits = hx71x->sps_bits;
} else {
extra_bits = hx71x->gain_channel;
}
uint32_t adc = hx71x_raw_read(hx71x->dout, hx71x->sclk, 24u + extra_bits);

// Clear pending flag (and note if an overflow occurred)
irq_disable();
Expand All @@ -156,12 +165,12 @@ hx71x_read_adc(struct hx71x_adc *hx71x, uint8_t oid)
irq_enable();

// Extract report from raw data
uint32_t counts = adc >> gain_channel;
uint32_t counts = adc >> extra_bits;
if (counts & 0x800000)
counts |= 0xFF000000;

// Check for errors
uint_fast8_t extras_mask = (1 << gain_channel) - 1;
uint_fast8_t extras_mask = (1 << extra_bits) - 1;
if ((adc & extras_mask) != extras_mask) {
// Transfer did not complete correctly
hx71x->last_error = SAMPLE_ERROR_DESYNC;
Expand All @@ -186,17 +195,23 @@ command_config_hx71x(uint32_t *args)
struct hx71x_adc *hx71x = oid_alloc(args[0]
, command_config_hx71x, sizeof(*hx71x));
hx71x->timer.func = hx71x_event;
uint8_t gain_channel = args[1];
if (gain_channel < 1 || gain_channel > 4) {
uint8_t gain_or_sps = args[1];
uint8_t sps_bits = args[2];
uint8_t gain_channel = args[3];
if (gain_or_sps == 0 && (gain_channel < 1 || gain_channel > 4)) {
shutdown("HX71x gain/channel out of range 1-4");
}
if (gain_or_sps == 1 && (sps_bits < 1 || sps_bits > 4)) {
shutdown("HX71x sample_rate out of range 1-4");
}
hx71x->gain_or_sps = gain_or_sps;
hx71x->sps_bits = sps_bits;
hx71x->gain_channel = gain_channel;
hx71x->dout = gpio_in_setup(args[2], 1);
hx71x->sclk = gpio_out_setup(args[3], 0);
hx71x->dout = gpio_in_setup(args[4], 1);
hx71x->sclk = gpio_out_setup(args[5], 0);
gpio_out_write(hx71x->sclk, 1); // put chip in power down state
}
DECL_COMMAND(command_config_hx71x, "config_hx71x oid=%c gain_channel=%c"
" dout_pin=%u sclk_pin=%u");
DECL_COMMAND(command_config_hx71x, "config_hx71x oid=%c gain_or_sps=%c sps_bits=%c gain_channel=%c"" dout_pin=%u sclk_pin=%u");

// start/stop capturing ADC data
void
Expand Down