rock-core / base-types

C/C++ and Ruby definition of the base types in Rock
6 stars 40 forks source link

Add Logmessage type #167

Closed planthaber closed 1 month ago

planthaber commented 4 months ago

Adds a LogMessage type (a timestamped string)

Provides ability to:

planthaber commented 4 months ago

Here is the PR for the orogen part: https://github.com/rock-core/base-orogen-types/pull/41

chhtz commented 4 months ago

I suggest using time instead of timestamp to be compatible with other samples and related tooling (in case there ever is a use case for that). And maybe simply base::TimeStamped<std::string> would work as well? (I haven't actually tried this)

planthaber commented 4 months ago

I suggest using time instead of timestamp to be compatible with other samples and related tooling (in case there ever is a use case for that). And maybe simply base::TimeStamped<std::string> would work as well? (I haven't actually tried this)

The TimeStamped template does not work for container types with gccxml:

<Thread:0x000057f977ce2200 /opt/workspace/install/lib/ruby/2.7.0/typelib/gccxml.rb:1043 run> terminated with exception (report_on_exception is true):

/opt/workspace/install/lib/ruby/2.7.0/typelib/gccxml.rb:656:in block (2 levels) in resolve_type_definition': undefined methodeach_field' for #<Typelib::ContainerType: /std/string> (NoMethodError)

But changed the time field

doudou commented 3 months ago

use a port to show/record the time of state changes

I would rather suggest we modify orogen to have a timestamped state change that uses the state enum. string is not hard real time, you can't make it mandatory at low level.

planthaber commented 3 months ago

I would rather suggest we modify orogen to have a timestamped state change that uses the state enum. string is not hard real time, you can't make it mandatory at low level.

Yes, but that was just an example why the type may be useful in general. The original reason I need the type is the text logging orogen plugin I mentioned (which is only needed for debugging anyways).

It already works, when the orogen plugin is active, stdout and stderr are split up into writing to the log file (as it is now) and additionally writing the content LogMessage to a port. This makes it possible to collect and display all produced text in a single application rather than searching though the log files in the bundle (including filtering on task/context level). But here is where we need the timestamp in order to at least have the possibility to stream align the messages.

Also, when the strings are set in the constructor or confiugure hook, it should be fine with the real-time contraint right? Then it's on the implementation side, and it is not mandatory at all.

On the plugin this is probably not real-time, but it is for debugging purposes only and can be deactivated when needed, it will be inactive when no output is written to stdout, anyways.

planthaber commented 2 months ago

If there are no further objections, I'll merge it soon.