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

[P026][Sysvars] Add Internal temperature sensor value for ESP32 #4820

Conversation

tonhuisman
Copy link
Contributor

@tonhuisman tonhuisman commented Sep 23, 2023

Resolves #4817

Features:

  • P026; Add Internal temperature (ESP32) as a value for ESP32 processors
  • System variables: Add %inttemp% (Internal temperature) for ESP32
  • Show correct branch name in Actions run builds
  • [Build] Stop Documentation-only commits on existing branches from triggering an Actions run

TODO:

  • Testing
    • ESP32 Classic (Currently works using IDF 4.4.5, but rumors are that it might be dropped from support)
    • ESP32 S2
    • ESP32 C3
    • ESP32 S3
  • Update documentation
  • Test: Documentation commit. Result: Not working as expected, (but still 'by design' 🤔 as described here

@TD-er
Copy link
Member

TD-er commented Sep 23, 2023

  • System variables: Add %internaltemperature% (Internal temperature) for ESP32

Couldn't you come up with an even longer name? I think this one can even be used on nearly all display plugins in their line strings. ;)

src/_P026_Sysinfo.ino Outdated Show resolved Hide resolved
src/src/Helpers/Hardware.cpp Outdated Show resolved Hide resolved
@tonhuisman
Copy link
Contributor Author

Couldn't you come up with an even longer name?

Shortened to %inttemp%

src/src/Helpers/Hardware.cpp Outdated Show resolved Hide resolved
@iz8mbw
Copy link
Contributor

iz8mbw commented Sep 24, 2023

Testing %inttemp% (HEAD_1f9e4a7) on ESP32, Temperature values alternates between "sense" values (likes 65 degrees) to -273,15 (no sense value):
image

image

@TD-er
Copy link
Member

TD-er commented Sep 24, 2023

I think the sensor needs some 'time' to stabilize as there is a check to see if the value returned is 128 and if so it will return the error value.

@TD-er
Copy link
Member

TD-er commented Sep 24, 2023

By the way, it looks like the temperature sensor on the ESP32 (classic) is just (barely) capable of giving proper readings.
So you should only use it to track trends.

I think it just tracks the offset in frequency of the internal 150 kHz oscillator (its accuracy is highly temperature dependent) compared to the crystal frequency.

The ESP32-S2/S3/C3/C2/C6 seem to have a more reliable temperature sensor, or at least have a factory calibration.

I think that's why Espressif removed the "temperature sensor" from the features summary of the ESP32 classic.

@TD-er
Copy link
Member

TD-er commented Sep 24, 2023

I just upload this PR into the 'latest' web flasher: https://td-er.nl/ESPEasy/latest/

@iz8mbw
Copy link
Contributor

iz8mbw commented Sep 25, 2023

I think that's why Espressif removed the "temperature sensor" from the features summary of the ESP32 classic.

So if on the classic ESP32 we cannot obtain acceptable values (-273.15 is unacceptable) I suggest to remove this feature for the ESP32 board.

@TD-er
Copy link
Member

TD-er commented Sep 25, 2023

Like I said, I think the temperature sensor may need some more polling as it will only return this extreme value when the sensor returns this specific value (128 if I'm not mistaken)

Still the temperature reading on ESP32 classic should not be used as a true temperature value. Only as a trend.

@iz8mbw
Copy link
Contributor

iz8mbw commented Sep 26, 2023

Just tested the last commit of this PR (via Action run https://github.com/letscontrolit/ESPEasy/actions/runs/6305109576) and I can confirm now all is OK and I don't have more any -273.15 value.
Thanks!

@TD-er TD-er merged commit de55dbb into letscontrolit:mega Sep 26, 2023
161 checks passed
@tonhuisman tonhuisman deleted the feature/P026-add-internal-temperature-also-as-systemvar branch September 26, 2023 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Add support for internal ESP32 temperature sensor
3 participants