EspLogger is not extensible

[gvaradarajan]

I would like to be able to extend the functionality of EspLogger for my own custom purposes like so

struct MyLogger<L>(L);

impl<L> ::log::Log for MyLogger<L>
where
    L: ::log::Log,
{
    fn enabled(&self, metadata: &log::Metadata) -> bool {
        self.0.enabled(metadata)
    }

    fn flush(&self) {
        self.0.flush()
    }

    fn log(&self, record: &log::Record) {
        if self.enabled(record.metadata()) {
            self.0.log(record);
            <do other things with the record...>
        }
    }
}

However as of this commit, I'm no longer able to do so because the public struct now has a private constructor. Could we maybe throw a cheeky "pub" in front of that const fn new line?

[ivmarkov]

Help me understand how a pub EspLogger::new would help. It is not like Rust is an OOP language, where you can "inherit" from EspLogger, so not sure how that would be useful?

(And btw the new is private because the logger is supposed to be a singleton.)

[gvaradarajan]

Help me understand how a pub EspLogger::new would help. It is not like Rust is an OOP language, where you can "inherit" from EspLogger, so not sure how that would be useful?

(And btw the new is private because the logger is supposed to be a singleton.)

Rust is not OOP, but composition is still possible. In the example I put above, let's suppose that the <do other things with the record> involves uploading the log to some cloud storage. I could call log::set_boxed_logger(MyLogger(EspLogger::new())) at the start of my code and then all Rust logs would be efficiently redirected to UART as EspLogger is implemented to do and then MyLogger could redirect that log over the internet with its own logic.

I understand the pattern of desiring a singleton, but enforcing that pattern is already handled by the log crate.

[ivmarkov]

Help me understand how a pub EspLogger::new would help. It is not like Rust is an OOP language, where you can "inherit" from EspLogger, so not sure how that would be useful? (And btw the new is private because the logger is supposed to be a singleton.)

Rust is not OOP, but composition is still possible. In the example I put above, let's suppose that the <do other things with the record> involves uploading the log to some cloud storage. I could call log::set_boxed_logger(MyLogger(EspLogger::new())) at the start of my code and then all Rust logs would be efficiently redirected to UART as EspLogger is implemented to do and then MyLogger could redirect that log over the internet with its own logic.

I understand the pattern of desiring a singleton, but enforcing that pattern is already handled by the log crate.

OK, so your idea would be:

• Wrap the esp logger in your own stuff
• Do NOT call EspLogger::initialize_default, and instead do your own ::log::set_logger(...) dance with the log crate

BTW: You do realize this way you can only grab the logs originating from rust code. Logs originating from the C code you need to grab with a different facility, which is much more hairy (vsprintf and stuff like that).

[gvaradarajan]

Yes that's exactly what my plan is. I am aware about those other logs, I have a separate additional use of esp_log_set_vprintf that handles those statements

[ivmarkov]

Yes that's exactly what my plan is. I am aware about those other logs, I have a separate additional use of the esp_log_set_vprintf that handles those statements

Would be great if you could share your code that deals with the vprintf blackmagic from Rust (if you do it from Rust and with pure rust (i.e. no C stubs and such)). I remember a user was fighting this for a long time without declaring success. We might include such an utility in esp-idf-svc::log as that might be useful to many folks.

[ivmarkov]

Closing as fixed by https://github.com/esp-rs/esp-idf-svc/commit/11d9d0a417335f6e1fb596f03bf53dc4050862f1

But if you could really share the vsprintf stuff, that would be helpful.

[gvaradarajan]

Thank you for hearing me out @ivmarkov! I have an example for using the vprintf setter that I wrote for my work here