Skip to content
This repository has been archived by the owner on Aug 27, 2023. It is now read-only.

Added support for MCP3008 and MCP3004 ADC for temperature read #205

Closed
wants to merge 0 commits into from

Conversation

wsowa
Copy link
Contributor

@wsowa wsowa commented Apr 7, 2016

I'm building custom 3d printer with custom electronics. Machine is based on ultimaker design while electronics on Gen7. I use MCP3008 8-channel 10 bit SPI ADC for temperature read instead of ADC build in AVR (I need to save pins). With this pull request I add support for this ADC to teacup. I hope someone else will make use of it.

Most changes are related to adding support for MCP3008 to python config tool rather than to actual teacup code so I suggest to start reviewing with C code and then go to python and configs

temp.c Outdated
do {
uint8_t j, table_num;
//Read current temperature
temp = analog_read(i);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a bug. Should be temp = analog_read(temp_sensors[i].temp_pin) because we want to read ADC pin of i-th sensor rather then ADC pin i. Fixed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Analog read is index based. So we don't read the temp_pin. We read the ADC of index i, which is configured in analog_xxx.c.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. You're right. I read analog part and somehow I thought that it reads pin it gets in argument directly. Reverted to analog_read(i)

Copy link
Owner

Choose a reason for hiding this comment

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

And then there is this thread: http://forums.reprap.org/read.php?1,647767,648074#msg-648074 Is this the reason why the existing code was suspect to you?

Other than that: nice patch! I'll apply it before too long.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't read this thread till now. The reason why I need separate ADC is that I have 5 temperature sensors in a print bed and 4 heaters. Print bed is 40x40cm with four 20x20 heaters. One sensor sits in the middle of each heater and the fifth one is in the middle of a test bed. You may see it in the video I tweeted some time ago: https://twitter.com/WitekSowa/status/713456028860473344
This number of sensors and heaters plus LCD, jog dial, SD and at least 5 endstops (max in Z axis, min/max in X/Y) requires much more pins than ATMega have so I run ADC/SD via SPI and LCD/Jog dial via I2C port multiplexer (MCP23017).

@Traumflug
Copy link
Owner

Good. Applied and squashed both commits to experimental with some whitespace editing and rearrangements (I'm a nitpicker :-) ). Regression tests pass.

Please test wether the code on experimental works for you, then we can close this pull request.

@wsowa
Copy link
Contributor Author

wsowa commented Apr 12, 2016

Will check in 2-3 weeks. I'm. On holiday now

@Wurstnase
Copy link
Collaborator

I have some issues to read that code.

The MCP is an ADC over SPI. So you have a CS, which you can choose in the configtool. This is all ok.

But for the channel you can only select some normal ADC-pins from the MCU not the channels from the MCP?!?

@wsowa
Copy link
Contributor Author

wsowa commented May 27, 2016

@Traumflug I've checked experimental branch. MCP3008 support works fine but I got into problems with pre-computed slope in thermistor table. See more details here: #208

@Wurstnase Yes, I reuse AIOx names for denoting ADC channels. AIOx translates just to x so it works fine. And I find it more user friendly: AIOx read as analog input x may refer to MCU channel or to ADC channel. There's no need to name them differently if user selects a sensor type as well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants