-
Notifications
You must be signed in to change notification settings - Fork 181
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
Missing support for internal temperature sensor #333
Comments
This repo is the right place to implement a safe wrapper around the raw C bindings. To roughly outline what would need to happen.
To dev it you just can generate a new project with the template make a fork of both this and the -sys repo and use [patch.crates-io]
esp-idf-sys = {path=<path to your local repo clone>} #or use a remote git repo
esp-idf-hal = {path=<path to your local repo clone>} PR's are welcome if you have any question check out our matrix channel |
Thanks for the quick reply, @Vollbrecht. I've tried to follow the steps you provided, but I am somewhat stuck. What I did so far is:
Now I am somewhat stuck, because I'm not sure if (and how) I need to create/update the bindings from
Perhaps you can explain the process in a little more detail and answer my questions and/or confirm if my assumptions about the process are correct. Thanks again! |
if you are using cargo generate for your project, the default behavior is that it clones and installes esp-idf all by itself in a project local dir called .embuild. so thats already covert. esp-idf-sys generates all the bindings and export them directly. They are several ways to get the complete output, you could
With this you can verify know that the functions defined in the esp-idf function header are now available. |
When are they generated? What triggers their generation? (As mentioned above I can't run
Does not exist in my
How can I find such functions? I tried to do what you mentioned (in VSCode) on various functions in other files but none of them "resolve" to
When I open the documentation, the only thing I see in regards to the temperature sensor are constants like From what I can see I suspect that the bindings are not generated. How can I generate them (manually)? |
the buildstep of esp-idf-sys generates them, (that's why you find them in the build-artifacts as i described it) inside your normal binary project that you created and if you use the crates-io patch it will build esp-idf-sys alongside with your local changes. |
To make it clear if your include worked you would for example see the struct |
Alright, thanks for the clarification. Sounds like my include was wrong then. I'll try again and report back. Sorry for beeing such a noob and thanks for the patience. |
Quick update before I go to bed: I got a temperature out of the ESP successfully. I'll try to create a PR in the next few days. |
As mentioned previously I got a temperature reading. I used this (very basic PoC) code to accomplish that: use std::{thread::sleep, time::Duration};
use esp_idf_sys::{
soc_periph_temperature_sensor_clk_src_t_TEMPERATURE_SENSOR_CLK_SRC_DEFAULT,
temperature_sensor_config_t, temperature_sensor_enable, temperature_sensor_get_celsius,
temperature_sensor_handle_t, temperature_sensor_install,
};
fn main() {
// It is necessary to call this function once. Otherwise some patches to the runtime
// implemented by esp-idf-sys might not link properly. See https://github.com/esp-rs/esp-idf-template/issues/71
esp_idf_svc::sys::link_patches();
// Bind the log crate to the ESP Logging facilities
esp_idf_svc::log::EspLogger::initialize_default();
let mut temp_sensor: temperature_sensor_handle_t = std::ptr::null_mut();
let temp_sensor_config = temperature_sensor_config_t {
range_min: 10,
range_max: 50,
clk_src: soc_periph_temperature_sensor_clk_src_t_TEMPERATURE_SENSOR_CLK_SRC_DEFAULT,
};
unsafe {
temperature_sensor_install(&temp_sensor_config, &mut temp_sensor);
temperature_sensor_enable(temp_sensor);
}
let mut tsens_value: f32 = 0.0;
loop {
unsafe {
temperature_sensor_get_celsius(temp_sensor, &mut tsens_value);
}
log::info!("{}", tsens_value);
sleep(Duration::from_millis(1000));
}
} I now wanted to "move" the code from my main method in my use esp_idf_sys::soc_periph_temperature_sensor_clk_src_t;
pub type Config = config::Config;
/// Temperature Sensor configuration
pub mod config {
#[derive(Copy, Clone)]
pub struct Config {
// TODO: check int size
pub range_min: i32,
pub range_max: i32,
pub clk_src: soc_periph_temperature_sensor_clk_src_t,
}
impl Default for Config {
fn default() -> Self {
Config {
range_min: -10,
range_max: 80,
clk_src: soc_periph_temperature_sensor_clk_src_t_TEMPERATURE_SENSOR_CLK_SRC_DEFAULT,
}
}
}
impl Config {
pub fn new() -> Self {
Default::default()
}
}
} and I'm trying to use the following in my main method:
The problem is that compilation fails because the import from esp_idf_sys cannot be found. I've added the path override you mentioned above to the esp-idf-hal project too and also tried using I'm now wondering how I can use the types from the bindings in esp-idf-hal and if I that is even the right approach. I don't know if what I'm writing for esp-idf-hal should "merely" be a wrapper with - perhaps - some nicer names for the generated bindings (which is what I currently think), or if I need to do "more". In addition I'm also wondering
But I think those questions can be answered later, when a first draft for a PR exists. |
Regarding the imports:
As you see - often - we implement our own types for our public API, and not re-export the original C types. This is then accompanied with You mention a good point with regards to "wrapper api" , while we essentially wrapping many of the underlying C drivers, our main goal is to be a good rust api and not just a thin safe wrapper. So we avoid to leak to much of the underlying C api if the user only directly uses hal/svc. That way we can support multiple esp-idf versions ( currently v4.4.6 and 5.1.1) with one api. But if something only exist for example in esp-idf 5 and onwards we can hide a complete api behind this if its necessary. A second goal is also to not use to much generics, but this should not be a problem here. A third goal is - as embedded lib developers - to limit our self by trying to avoid allocation heavy stuff and try to make parts work in no_std environments ( even if the complete project could not be used in a no_std env) At compile time you got the information handled downward with compiler flags, so you can determine if the current api is compiled against esp-idf 5.1 or 4.6. Also you got information regarding what type of esp is used. Check out some of the other drivers and you find examples quickly |
So If I understand you correctly I should use
but the import does not resolve. As far as I can tell the same approach works for the other "drivers" (e.g. for Any ideas what I could be doing wrong and how I could debug it? Thanks also for the clarifications regarding my additional questions - I'll keep them in mind when writing my code. Thanks again for your help! |
Still make sure you don't pull the multiple versions of esp-idf-sys. To make your life a little simpler, here is how you can compile esp-idf-hal directly ESP_IDF_VERSION="v5.1.1" MCU="esp32c3" cargo build --target riscv32imc-esp-espidf --features=nightly,criticalsection,embassy-sync,embedded-hal-async -Zbuild-std=std,panic_abort -Zbuild-std-features=panic_immediate_abort you just adjust the idf version, mcu, flags and target to your testcase. For your local testing you can add a .cargo/config.toml, so you dont need to to run this long command every-time inside esp-idf-hal, similar to your bin crate but we don't want to have this in a potential PR from you. With this you also could run Edit: you also might need to set the correct rust toolchain, so maybe need to add +esp if its not the default one |
Sorry, forget to mention. Output of the cargo tree command was/is
as far as I can tell this is as expected. esp-idf-hal depends on esp-idf-sys and since there is only one line with I also tried your command (I had to use
Interestingly I also get
The types do exist in the bindings at Any ideas? |
Is this an rust-analyzer error or this error comes up when you try to compile it? |
when working with local repos and git repos also always try to use |
Errors come up when trying to build with
I did execute
No, they were not. I committed them, but the issue persists. I also suspect committing does not make a difference since the bindings were correctly generated. |
you are currently developing it for the esp32 right? so with my command you are building it for the esp32c3, and this mcu don't have this driver generated inside esp-idf-sys because it does not have this driver / internal hardware ! So make sure it works for the esp32 and generate fine there, and also you can build it for othere targets. (You need to gate the imports to the targets where they exist !! ) So esp32 is |
No, ESP32C3 (atm; long term goal is for it to work on all esp32's that have the internal temp-sensor). Sorry, should've probably clarified earlier.
I don't quite understand what I would have to do here - can you give an example? |
Yeah sorry to make it clear. If you are generating the bindings only for a special target -> #if defined(CONFIG_IDF_TARGET_ESP32)
#include "driver/temperatur_sensor.h"
#endif you than only can use any statements regarding this api only behind similar gatex expression, So if you are using |
Even if its not currently able to build you can create a draft PR, so i can see what you are doing more clearly. 😸 |
Sure, I'll quickly do that. |
Various ESP32 models have an integrated temperature sensor that is supported by esp-idf.
It would be nice to see support for that sensor built into this crate.
I would like to try implement it - what steps need to be taken for such an implementation? Is there documentation for that process? Is this crate even the right place or does it need to be implemented in esp-idf-sys instead?
Links
The text was updated successfully, but these errors were encountered: