Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extras/ads1x1x.py: Add support for i2c based ADC chip #5618

Closed
wants to merge 1 commit into from

Conversation

kpishere
Copy link

ADC1X1X is a series of i2c bus chips with high sample rate and multiple channels.
Each channel can be mapped to a heater device and use the existing sensor math
and/or lookup tables. Documentation is added. A sample configuration is also
included (printer-picksix-v02.cfg).

Signed-off-by: Kevin Peck kevindpeck@gmail.com

ADC1X1X is a series of i2c bus chips with high sample rate and multiple channels.
Each channel can be mapped to a heater device and use the existing sensor math
and/or lookup tables.  Documentation is added.  A sample configuration is also
included (printer-picksix-v02.cfg).

Signed-off-by: Kevin Peck <kevindpeck@gmail.com>
@KevinOConnor
Copy link
Collaborator

I'm not really sure what this PR does. It seems to have several different topics combined into one PR (pt100, a printer config file, changes to adc handling, a new temperature sensor, whitespace changes, removal of reserved pin check, etc.). Alas, this is too complicated for me to review. Changes should use git rebase and have one atomic change per commit - this is important as it makes it easier to review and also revert if a regression is found. See https://www.klipper3d.org/CONTRIBUTING.html .

Separately, there has been interest in the ads11xx chips for use in load cell monitoring - see https://klipper.discourse.group/t/strain-gauge-load-cell-based-endstops/2134 and https://klipper.discourse.group/t/pr-introduce-load-cell-based-probe/1526 for example. I think there should be some consensus on how these chips should be handled in Klipper prior to merging support.

-Kevin

@KevinOConnor KevinOConnor added the pending feedback Topic is pending feedback from submitter label Jul 6, 2022
@kpishere
Copy link
Author

kpishere commented Jul 6, 2022

Yes, you'd listed all changes, not sure what else with ".etc".

pins.py: That configuration check had to be removed because this device has up to four sensors at one set of gpio pins and address and it wasn't allowing the use of the same pins in a heater and in an extruder device.

adc_temperature.py: The PT100 related calculation changes, I think in the end I didn't use those, they can still be used but I can remove as they are "un-tested", well, they weren't the right calculation method for the sensor I'd used.

The config file is a working configuration example. Up to you if you'd want it. I'd figure some like to see it as example of working configuration to complement the configuration instructions.

Besides the above, all components in this commit allow you to wire the ADC and use any of its up to four sensors for a controlled heater device like extruder, bed heater, or enclosure heater as you would for any direct ADC GPIO pin on an MCU.

The ADC does MUX switching and then sampling in separate steps, so sampling ended up being a little more complicated as the switch and sample operations are open ended and you don't want to sample a value to the wrong channel. Measuring an extruder temperature on a bed heater would shut down the MCU immediately. So a singleton round-robin state machine was needed to control MUX 1, Sample, MUX 2, Sample, MUX 3, Sample, MUX 4, Sample, MUX1, .... kind of pattern in regular intervals.

Other driver implementations may want to take advantage of polling or interrupt use in these devices at higher sample rates but this driver is not intended to satisfy that, it is for low samples per second even though they handle much higher sample rates.

@dockterj
Copy link

I posted some questions in discourse that are related to this:
https://klipper.discourse.group/t/multiplexed-devices-how-to-configure-two-temperature-sensors-on-the-same-adc/3502

@github-actions
Copy link

github-actions bot commented Aug 9, 2022

It looks like this GitHub Pull Request has become inactive. If there are any further updates, you can add a comment here or open a new ticket.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

@github-actions github-actions bot added the inactive Not currently being worked on label Aug 9, 2022
@github-actions github-actions bot closed this Aug 9, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Aug 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
inactive Not currently being worked on pending feedback Topic is pending feedback from submitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants