esp-rs / esp-idf-sys

Bindings for ESP-IDF (Espressif's IoT Development Framework)
Apache License 2.0
275 stars 124 forks source link

va_list: core:ffi:VaList instead of [u32; 3] #212

Closed thomas725 closed 10 months ago

thomas725 commented 1 year ago

I would like to send esp-idf log output through my own code to a syslog server, instead of "just" to the UART0 serial line (=default behavior)

It looks like this interface: https://esp-rs.github.io/esp-idf-sys/esp_idf_sys/fn.esp_log_set_vprintf.html should enable me to do this.

VS-Code lead me to the file target/xtensa-esp32-espidf/debug/build/esp-idf-sys-d049f0656ab9d370/out/bindings.rs which contains:

pub type va_list = [u32; 3usize];
pub type vprintf_like_t = ::core::option::Option<
    unsafe extern "C" fn(arg1: *const ::core::ffi::c_char, arg2: va_list) -> ::core::ffi::c_int,
>;
extern "C" {
    pub fn esp_log_set_vprintf(func: vprintf_like_t) -> vprintf_like_t;
}

What I'm missing, is a way to join the format pattern with its va_list arguments, which sounds like the same problem that @bjoernQ was having here: https://github.com/esp-rs/esp-wifi/issues/16

This crate looks like it would be a very nice solution: https://docs.rs/printf-compat/latest/printf_compat/ but sadly that would require the va_list type to be core::ffi:VaList, not [u32; 3] - and I couldn't find any way to convert it. See also: https://users.rust-lang.org/t/esp-idf-logger-redirection-vprintf-variadic-function/95568

MabezDev commented 1 year ago

You can most likely safely transmute [u32; 3] to VaList because the Xtensa implementation is Repr(C) with only three words as fields: https://github.com/esp-rs/rust/commit/60712cbbd929c1d5ffd4371853ced2ee034e9388#diff-92ac4f00ad02a9a9d7ebcd5337dec510097384904750feee32110bd614500050R384.

thomas725 commented 1 year ago

thanks for the reply! can you share the syntax how to do that transmutation?

ivmarkov commented 1 year ago
let va_list: [u32; 3usize] = ...;
let va_list: VaList = core::mem::transmute(va_list);
thomas725 commented 1 year ago

sadly, that results in a compiler error for me:

cannot transmute between types of different sizes, or dependently-sized types
source type: `[u32; 3]` (96 bits)
target type: `VaList<'_, '_>` (32 bits)
ivmarkov commented 1 year ago

Try transmuting &[u32; 3] then?

thomas725 commented 1 year ago

Thanks! I've tried both these options:

let mut args: VaListImpl = core::mem::transmute(args);
let mut args: VaList = core::mem::transmute(&args);

but passing it allong to the printf-compat crate (for the VaListImpl by using args.as_va_list()) just as described in their intro page:

format(pattern, args, output::fmt_write(&mut string));

for me causes this runtime error:

Guru Meditation Error: Core 0 panic'ed (LoadProhibited). Exception was unhandled.

bjoernQ commented 1 year ago

You most probably hit the same problem I have:

https://github.com/esp-rs/rust/issues/177

thomas725 commented 1 year ago

I wonder if there maybe is some way to use the C code that the default esp-idf log handler uses to combine the pattern with its va_list arguments...

bjoernQ commented 1 year ago

I thought about writing the logging implementation in C (e.g. using some of the ROM functions for string formatting) and then call into Rust with just the formatted string as a parameter - that should definitely work

thomas725 commented 1 year ago

though I guess to be able to also write C code, not only rust code, you'd need to have a different project structure setup, right?

ivmarkov commented 1 year ago

There's an in-progress PR against esp-idf-sys which is for something else, but the bottom line is that this PR is introducing facilities to compile a piece of custom C code together with the rest of esp-idf-sys, where this separate C code is not really an ESP-IDF component. Once we have this "custom C code facility", we can also plug-in there a C stub for vfprintf which maybe can call a user-supplied Rust function.

Another approach might be to implement the C code as a custom ESP IDF component and wrap it with a Rust crate. This is in fact kind of supported already now: https://github.com/esp-rs/esp-idf-sys/blob/master/build/native/cargo_driver/config.rs#L285

thomas725 commented 1 year ago

wow, those are amazing insights, thanks!

If you get around to actually implement this or find time to give me some more pointers that could enable a n00b like me to do it myself, I'd love to read more from you :)

MabezDev commented 10 months ago

FYI Xtensa vaarg has been fixed since https://github.com/esp-rs/rust/pull/201, sorry for forgetting about this issue!

thomas725 commented 10 months ago

@MabezDev thank you for the notice! I've uncommented my code snippet that was causing this

Guru Meditation Error: Core 0 panic'ed (LoadProhibited). Exception was unhandled.

runtime panic before (and hid it behind a config that would only auto-enable on boots that weren't caused by a panic - see commit )

and it works perfectly :partying_face: