Open wouterdebie opened 1 year ago
Yeah. These are the dangers of using glob-imports (as in use esp_idf_sys::*
). As the content of the module you are importing might change, you are risking name conflicts. In that particular case, it seems the esp-cam
component has a static named... resolution
. If that's not clear, this is not your fault, but ours, as we were lazy (me specifically) and we were glob-importing the esp-idf-sys
namespace.
No easy fixes :(
esp-idf-sys
anymore. Anywhere. This means, replace - in esp-idf-hal
and esp-idf-sys
- use esp_idf_sys::*
by use esp_idf_sys as esp
and then prefix all calls to functions in esp-idf-sys
with esp
.esp-cam
should be in esp_idf_sys::esp_cam
or better yet - esp_idf_sys::ext_components::esp_cam
. Pros and cons whether this should be the default behaviorI think we need to actually do all of 1, 2 and 3, and perhaps in the following order: 2, then 3, then 1. Or 3, then 2 then 1.
Given how easy option 2 is, perhaps you can do it? The line that we fixed (removed) yesterday should be very near to where you need to do a code change.
Oh, and you can of course always just temporarily patch esp-idf-hal
's adc
module and rename the resolution
var in there to resolution2
or something, but I'll of course never upstream such a patch. :)
Thanks for this! In my case option 2 is probably the best. How is this different from using bindings_module
in the extra_components
? (which doesn't seem to solve the problem)
Another option would be to patch the esp-camera
component itself and rename resolution
there. I actually do this already since esp_camera.h
contains a union
that leads to an anonymous struct member by bindgen
. Is there an easy way of patching source code before building? Right now I'm doing this manually, but since I'd like esp-camera
to be a submodule, rather than vendored code, I'd love to do this automatically.
Thanks for this! In my case option 2 is probably the best. How is this different from using
bindings_module
in theextra_components
? (which doesn't seem to solve the problem)
No idea, as it is not me who wrote the components support originally, so now I need to read the code of it, just like you do.
Another option would be to patch the
esp-camera
component itself and renameresolution
there. I actually do this already sinceesp_camera.h
contains aunion
that leads to an anonymous struct member bybindgen
. Is there an easy way of patching source code before building? Right now I'm doing this manually, but since I'd likeesp-camera
to be a submodule, rather than vendored code, I'd love to do this automatically.
Just fork esp-camera
then commit the fixes to your repo and then depend on your repo instead of on the original one. The cleanest approach IMO.
Regarding bindings_module
I saw your comment in https://github.com/esp-rs/esp-idf-sys/issues/182. However, when doing this (bindings_module = "esp_cam")
, I'm getting the following warning:
warning: `sigaction` redeclared with a different signature
--> /Users/wouter.de.bie/prjs/wouter/esp32-cam-rs/target/xtensa-esp32-espidf/debug/build/esp-idf-sys-d65a20c05d8c81d5/out/bindings.rs:69163:9
|
17728 | / pub fn sigaction(
17729 | | arg1: ::core::ffi::c_int,
17730 | | arg2: *const sigaction,
17731 | | arg3: *mut sigaction,
17732 | | ) -> ::core::ffi::c_int;
| |____________________________- `sigaction` previously declared here
...
69163 | / pub fn sigaction(
69164 | | arg1: ::core::ffi::c_int,
69165 | | arg2: *const sigaction,
69166 | | arg3: *mut sigaction,
69167 | | ) -> ::core::ffi::c_int;
| |________________________________^ this signature doesn't match the previous declaration
|
= note: expected `unsafe extern "C" fn(i32, *const bindings::sigaction, *mut bindings::sigaction) -> i32`
found `unsafe extern "C" fn(i32, *const esp_cam::sigaction, *mut esp_cam::sigaction) -> i32`
= note: `#[warn(clashing_extern_declarations)]` on by default
No idea, as it is not me who wrote the components support originally, so now I need to read the code of it, just like you do.
Understand :) As it seems to be narrowed down to a warning, so I'm getting somewhere :) Let me have a look to see if I can figure out what's going on here and how to fix this.
- Option 2: Provide a way to put the symbols of the custom component in a separate module. As in -
esp-cam
should be inesp_idf_sys::esp_cam
or better yet -esp_idf_sys::ext_components::esp_cam
. Pros and cons whether this should be the default behavior
Maybe I can chime in for this:
When I implemented this, I also wanted to put component's bindings in different modules, but it was almost impossible (at least in a non-hacky way) to implement. And as far as I can tell this is still true today (though maybe it is possible by the new bindgen::ir
module (I've only skimmed to docs a bit)).
To implement this, you would need to know which C header the bindings came from during generation and add the appropriate code to the generated bindings. There is a CargoCallbacks::include_file
callback but it doesn't allow to return code to be added to the bindings.
This is the reason that separate bindgen instances are run to put extra component's bindings in a separate module (and explains the docs on the bindings_module
parameter).
Hopefully, this explanation shed some light on this.
@N3xed thanks for the context! I didn't realize that extern "C"
functions can still clash, even though they are in separate modules, until I read https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/builtin/static.CLASHING_EXTERN_DECLARATIONS.html.
I'm working on a project that is based on
[esp-idf-template](https://github.com/esp-rs/esp-idf-template)
and when I'm trying to includeesp-idf-hal
in myCargo.toml
compilation fails with the following error:I'm compiling on MacOS with ESP IDF v5.0.
Any idea what is wrong?