re: [Add support for i2c based ADC chip by kpishere · Pull Request #5618 · Klipper3d/klipper · GitHub] (extras/ads1x1x.py: Add support for i2c based ADC chip by kpishere · Pull Request #5618 · Klipper3d/klipper · GitHub)
I’m posting this here instead of as a discussion point on the above PR because my use case is similar (and the chip is from the same family) but with enough differences that I didn’t want to pollute the PR discussion.
For background I have my Makerbot Replicator2 working with Klipper (code is POC, not ready for review) and part of what needed to be created was support for an ADS1118 ADC. There are two main differences in what I need to implement and the above PR:
1 - ADS1118 is SPI, the above PR is for i2c chips.
2 - The Rep2 uses a k type thermocouple instead of a RTD or thermistor.
3 - Supporting a Replicator2X will require the same ADS1118 chip to support readings on two different hot ends.
The challenges of handling a thermocouple are that the adc needs to sample the junction voltage and the temperature of the cold junctions need to be known. With those two measurements the junction temperature can be calculated (via polynomial or linear interpolation on lookup tables). The ADS1118 can sample two thermocouple voltages and has an internal temperature sensor to measure the cold junction temperature. (For single ended inputs this chip has four inputs plus the internal temperature).
The ADS1118 (and the other members of this family) aren’t powerful enough to gather readings from multiple mux configurations on their own. It can be programmed to continuously sample one input so when read you always get the latest complete sample. However, in order to get samples from multiple inputs the adc needs to be controlled with a simple state machine to start an adc conversion on the next sensor desired. The data returned from that will be the conversion from the last command. E.g. Current state is thermocouple1, send command to start conversion for thermocouple2 and read the returned data as the voltage for thermocouple1 and set the state to thermocouple2. On the next iteration, state is thermocouple2, so command reading the cold junction temperature and read the returned data as the voltage for thermocouple2 and set the state to cold_junction. Etc.
There’s a couple of architectural decisions that I ran into (and some are seen in the above PR).
Config issues
I can’t find an example in Klipper of a multiplexed configuration. For example, a Rep2X could look like this:
[extruder]
Sensor_type: ADS1118
spi_software_miso_pin: PE7
spi_software_sclk_pin: PE2
spi_software_mosi_pin: PH2
[extruder1]
Sensor_type: ADS1118
spi_software_miso_pin: PE7
spi_software_sclk_pin: PE2
spi_software_mosi_pin: PH2
A couple of issues with this approach:
1 - Klipper balks at having the same pins specified in two extruders. If the pins are left off of the second extruder config Klipper complains that the pins are missing for the second sensor.
2 - Even though the antiquated Rep2(X) is likely the only hardware implementation this will ever be used for I don’t like the amount of things assumed here. Assumption is that extruder is muxed to AIN0 and AIN1 and extruder1 is muxed to AIN2 and AIN3, another assumption is that these are thermocouples so a third measurement (cold junction) needs to be taken, and lastly we are assuming they are k type.
This seems bit better:
[temperature_sensor ADS1118_1]
spi_software_miso_pin: PE7
spi_software_sclk_pin: PE2
spi_software_mosi_pin: PH2
Thermocouple_type: k
[temperature_sensor ADS1118_2]
spi_software_miso_pin: PE7
spi_software_sclk_pin: PE2
spi_software_mosi_pin: PH2
Thermocouple_type: k
[extruder]
Sensor_type: ADS1118_1
[extruder1]
Sensor_type: ADS1118_2
Still some issues:
1 - Klipper would need to allow an extruder configuration with no sensor pins.
2 - ADS1118_1 and ADS1118_2 need to be handled by a singleton, so the connection between the two would be by naming convention (relying on _1 and _2 being the same physical adc).
This is what I liked more as it seems aligned with the hardware:
[temperature_sensor my_ADS1118]
adc_type: ADS1118
spi_software_miso_pin: PE7
spi_software_sclk_pin: PE2
spi_software_mosi_pin: PH2
sensor_type: k
Channels: 2
[extruder]
Sensor_type: my_ADS1118
Sensor_channel: 1
[extruder1]
Sensor_type: my_ADS1118
Sensor_channel: 2
It still hardcodes the mux settings but fixing that would be simple when a use case comes along. I think the Channels: 2 parameter can be eliminated by having the ADS1118 code scan the config file to get a list of configured sensors. Or instead of sensor_channel: each extruder could have the mux settings for it’s sensor (e.g. sensor_mux: 011 for the second channel).
However Klipper will need to be changed to allow the same sensor_type in this case for multiple extruders. This doesn’t seem clean in that the extruder class will need to know that my_ADS1118 is implemented by ADS1118 and in that case as long as the sensor_channels are different the config is valid.
Use case for load cells
Assuming the above approach works the code could be separated so the temperature_sensor with type of ADS1118 has the temperature reading code and a load_cell_probe would have some different configuration (sample rate being a good example).
[load_cell_probe my_load_cell]
Sensor_type: ADS1118
spi_software_miso_pin: PE7
spi_software_sclk_pin: PE2
spi_software_mosi_pin: PH2
Sensor_mux: 011
(I haven’t thought this through completely but the code should separate cleanly)
I’m open to all criticism or ideas. I’m happy to implement this any way that makes sense.