Skip to content
This repository was archived by the owner on Sep 6, 2023. It is now read-only.

esp32/esp: read internal temperature sensor #192

Closed
wants to merge 1 commit into from
Closed

esp32/esp: read internal temperature sensor #192

wants to merge 1 commit into from

Conversation

mogenson
Copy link

@mogenson mogenson commented Oct 2, 2017

Add a function to the esp module to read the ESP32's internal temperature sensor. This uses temprature_sens_read()[1] from the librtc.a library linked from the ESP-IDF.

I stuffed this into the esp module because it doesn't seem significant enough to warrant a new class in the machine module. Additionally, it could be seen as a property of the ESP32 module, like flash size.

This pull request is primarily a means for me to learn how micropython interacts with the ESP-IDF. Let me know if anything has not been implemented properly.

[1] misspelling is present in the ESP-IDF

Add a function to the esp module to read the internal temperature sensor
via temprature_sens_read(), located in librtc.a
@nickzoic
Copy link
Collaborator

nickzoic commented Oct 2, 2017

Great! I'll have a look as soon as possible ... I agree that this is probably an esp thing.

thinks unless most of the different platforms have something similar available in which case perhaps we should add a machine.get_internal_temperature to the common API.

@nickzoic
Copy link
Collaborator

nickzoic commented Oct 2, 2017

stm32 port has this as pyb.ADC(...).read_core_temp()

I didn't notice any of the other platforms having similar functionality, and the ESP32 interface is different to this one, so not sure how relevant that is!

@mogenson
Copy link
Author

mogenson commented Oct 2, 2017

If I understand correctly, the temperature sensor is read using the SAR ADC1. However, it's not listed as one of the ADC channels (like in the STM32), so it's not part of the ADC driver in the ESP-IDF.

I don't think the organization of micropython has to exactly mirror the ESP-IDF. It's probably a question of where a micropython user would look to find this peripheral.

There's also a hall effect sensor that's read using the ADC but not part of the ADC driver. When implemented, it's placement should be related to the placement of the temperature sensor.

Should these two sensors be methods of the machine.ADC class or go somewhere else?

@tuupola
Copy link

tuupola commented Oct 2, 2017

FWIW I think machine should only contain stuff which is available in all or most ports. Ie. ESP specific stuff should go to esp.

@nickzoic
Copy link
Collaborator

nickzoic commented Oct 3, 2017

Yeah, I agree. I wanted to check if 'most' other ports had an equivalent, in which case we should standardize something under 'machine' but it looks like not so I think 'esp' is fine.

@dpgeorge
Copy link
Member

dpgeorge commented Oct 5, 2017

@mogenson it looks like the temprature_sens_read() function is undocumented... is that right? What is the actual return value, is it in degrees Celcius?

Regarding the placement of the function: to maintain compatibility with the ESP8266 I think it would be best to create a new esp32 module for things very specific to the ESP32. For example there's already gpio_matrix_in() and gpio_matrix_out() which could be moved to an esp32 module.

@mogenson
Copy link
Author

mogenson commented Oct 5, 2017

@dpgeorge temprature_sens_read() is undocumented. It appears to return raw ADC counts.

So far the temprature_sens_read() function and example routine in esp-idf/components/esp32/test/test_tsens.c are the only ways to access the built in temperature sensor. I was interested in this sensor because it's part of the ESP32 and doesn't require assembling any extra hardware.

@nickzoic
Copy link
Collaborator

nickzoic commented Oct 6, 2017

Hmmm, I see what you mean, it's really truly undocumented in the sense that it isn't even in a header file.

Based on comments here espressif/esp-idf#146 I think we should avoid using it as it isn't documented and is spelled wrong ... perhaps we'd be better off reproducing the test_tsens.c code or pleading with Espressif to implement and document this function for us!

Also that code seems to be in two bits: firstly powering up the sensor, then reading it out. Our API should probably reflect this.

@dpgeorge
Copy link
Member

dpgeorge commented Oct 6, 2017

Based on comments here espressif/esp-idf#146 I think we should avoid using it as it isn't documented and is spelled wrong ... perhaps we'd be better off reproducing the test_tsens.c code or pleading with Espressif to implement and document this function for us!

Agreed.

@mogenson If you really want to use the sensor at this stage then it's possible to control the registers directly via the machine.mem32[] memory accessor (or uctypes module). You can write a little Python class to turn it on and read (and convert) a value, and that class should continue to work no matter what other changes there are to the API, either on the IDF level or the uPy level.

@mogenson mogenson closed this Oct 6, 2017
@nickzoic
Copy link
Collaborator

nickzoic commented Oct 7, 2017

Hi Michael, thanks for your contribution though and yes, you're going about it the right way.

Hopefully the underlying API issues will be resolved soon and we can revisit this then (see #187 where it says "We have a task for adding an API for temperature sensor, which will likely be addressed for the next major version")

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.

4 participants