rust-lv2 icon indicating copy to clipboard operation
rust-lv2 copied to clipboard

Log

Open YruamaLairba opened this issue 5 years ago • 14 comments

I started to work on the log spec.

YruamaLairba avatar Mar 29 '20 14:03 YruamaLairba

Ah this looks like the solution to #91. Could you add a usage example (perhaps in the dummy unit test)?

Philipp91 avatar May 13 '21 10:05 Philipp91

Ah this looks like the solution to #91. Could you add a usage example (perhaps in the dummy unit test)?

sorry, this branch is far behind the rest of the main branch. I don't remember the details, but i stopped this branch because at the moment, there is no way to implement fully an properly in pure rust. If you need it to display some debug information during plugin development, you can use "println!" instead and run the host from a console.

YruamaLairba avatar May 13 '21 20:05 YruamaLairba

Thanks!

Philipp91 avatar May 13 '21 21:05 Philipp91

Hi @YruamaLairba , do you still want to work on this PR?

While I agree full realtime formatting support is not easy (and, in fact, may very well be the job of a whole separate crate), I believe a simple function that just sends a nul-terminated string as a log to the host would be quite useful already. We can add the formatting part later. 🙂

prokopyl avatar Aug 09 '21 11:08 prokopyl

Lol, i read the Log spec API again, and in general, this should not be used in real-time context (does the doc have been updated ?). The only exception is for debugging purpose (trace) where messing real-time is not a concern. So formatting from plugin side instead of host side doesn't seems to be a real issue. I also think it's acceptable to support trace in real-time context through an optional feature. i think i will look at it soon since only few change are required

YruamaLairba avatar Aug 09 '21 12:08 YruamaLairba

Yes, I agree, realtime formatting doesn't seem like a very big concern for now. ^^ I think having a simple API like the one you have implemented already that just logs a C string, and letting users in charge of formatting/acllocations should be enough, as I believe most errors would happen during instantiation or other non-realtime contexts.

prokopyl avatar Aug 09 '21 16:08 prokopyl

@prokopyl wait a minute, i just looked in again and it appear i have some unpushed work in my local dir. I will do a push force with my work on the top the develop branch for a cleaner git history.

I also reminded a big interrogation i had about threading class of this feature. This feature is allowed in any non-realtime context, i think this implies the "worker" thread, but i really don't see how to implement that without messing the principle of separation of LV2 extension.

YruamaLairba avatar Aug 10 '21 19:08 YruamaLairba

Sorry, rebasing was a very bad idea, i didn't see the code wasn't compiling at all after therebase

YruamaLairba avatar Aug 10 '21 19:08 YruamaLairba

except documentation, i don't see what more i can do...

YruamaLairba avatar Aug 10 '21 21:08 YruamaLairba

@prokopyl have you an opinion on code i did?

YruamaLairba avatar Aug 11 '21 14:08 YruamaLairba

@prokopyl have you an opinion on code i did?

@YruamaLairba Yes! Sorry for the wait, I just did another review. :slightly_smiling_face:

Also, don't bother with making extensive documentation for now, I'll merge this into a log branch on this repo so that it can be written later!

prokopyl avatar Aug 16 '21 15:08 prokopyl

I think it's almost, it's only need documentation.

I wouldn't do submodule, there is too few type in my opinion.

YruamaLairba avatar Aug 20 '21 00:08 YruamaLairba

I completed and reworked the documentation.

I also removed the unsafety of the Entry trait. This is because the Log spec allow to define and use additional log level (i missed this earlier). I think this imply host should well behave when attempting to log with an unrecognized urid. I also add the URIbound as trait bound for Entry.

YruamaLairba avatar Aug 21 '21 17:08 YruamaLairba

@prokopyl you may didn't see, i'm ready for a review since a while.

YruamaLairba avatar Oct 01 '21 22:10 YruamaLairba