POETSII / Orchestrator

The Orchestrator is the configuration and run-time management system for POETS platforms.
1 stars 1 forks source link

Support for compile-time and run-time selection of handler_log level #258

Closed m8pple closed 3 years ago

m8pple commented 3 years ago

Changing the handler_log level at compile-time has been really useful for two reasons:

This would broadly correspond to changing: https://github.com/POETSII/Orchestrator/blob/fd69625bf05313e163d848bb60c0aa7c216a312c/Source/Softswitch/inc/softswitch.h#L35

to:

#define handler_log(level, ...)   if(level <= P_MAX_LOG_LEVEL){ __handler_log(deviceInstance->deviceIdx, level, __VA_ARGS__) }

Then some method is needed for the user to specify the max log level, and then inject a compile-time constant P_MAX_LOG_LEVEL into the handler.

The ability to set the log level at run-time is also useful, but with the orchestrators architecture it may make sense to only be able to specify the compile-time version for the compose.

This is a feature, but has previously been very valuable in practise.

It can be worked around by the user employing the #ifdef approach in the XML, though this still requires a constant in the SharedCode to be changed, rather than using a launch or compile-time parameter.

m8pple commented 3 years ago

Chasing the code for P_LOG_LEVEL through a number of different indirections, it seems there is an undocumented command or option logl, set here:

https://github.com/POETSII/Orchestrator/blob/fd69625bf05313e163d848bb60c0aa7c216a312c/Source/OrchBase/Handlers/CmComp.cpp#L527

I guess it is to compose?

Looks like that could be a good route through to compile-time filtering.

heliosfa commented 3 years ago

Compile time specification is already possible.

The log-level check is seven lines below the handler_log macro in softswitch.h within the templated __handler_log method:

    if(level >= P_LOG_LEVEL) softswitch_trivial_log_handler(src, pkt);

The check exists here because __handler_log is used internally within the softswitch where deviceInstance->deviceIdx does not exist directly, and these still need the log level specification.

Chasing the code for P_LOG_LEVEL through a number of different indirections, it seems there is an undocumented command or option logl, set here:

It is in the Softswitch/Supervisor/Composer documentation on page 33:

compose /logl = *,n

It is not listed in the UserGuide (as that is meant to be the "most useful" commands), but we can add it there.

m8pple commented 3 years ago

I searched in the orchestrator repo and the user-facing docs, but couldn't see anything about logl.

Searching through (what I thought) was the Vol III documentation I didn't see anything either. My impression was also that Vol III was internal, rather than user facing.

I'm not sure I've ever seen a document called Orch_Vol_IIIB. I can follow the link, but am not sure how to access it "normally". It has all kinds of information that I'd never heard of (or at least haven't come across in the last couple of months), which seems to be quite user-facing, rather than design details.

It doesn't seem to be in the repo on either development or 1.0.0-alpha (at least as far as I can tell), and isn't in the current release. Where is it?

If I follow the "releases" path, then none of the documents from the volumes directory are included, and they are only visible if you choose to browse the source, so they appear more as design documents (and self-describe as such), rather than user-facing docs.

heliosfa commented 3 years ago

I'm not sure I've ever seen a document called Orch_Vol_IIIB. I can follow the link, but am not sure how to access it "normally". It has all kinds of information that I'd never heard of (or at least haven't come across in the last couple of months), which seems to be quite user-facing, rather than design details.

It doesn't seem to be in the repo on either development or 1.0.0-alpha (at least as far as I can tell), and isn't in the current release. Where is it?

Sorry, I had forgotten to put it in the Volumes directory in the docs repo when #172 was merged (it was originally linked as and reviewed as part of that PR as it is in docx rather than markdown). The file is now in Volumes on the development branch of the docs repo.

My impression was also that Vol III was internal

Vol III is meant to be more developer facing - it details all of the nuts and bolts, design rational, full command sets, stuct layouts, data structure, etc. etc. Most of the docs repository can be thought of as part of Vol III, just broken out into smaller files that address specific parts.

Thinking about things, the full Composer command set should probably be replicated in the User Guide (Vol IV). I had only done some of the commands as they were the most commonly used.

mvousden commented 3 years ago

Nothing wrong with putting compose /logl in the user guide - would be a good thing to do. I put in that "most commonly used" clause mostly as an ass-covering caveat anyway!

heliosfa commented 3 years ago

The user guide has been updated with the logl and logh commands. Does this resolve your concerns?

heliosfa commented 3 years ago

Are you happy for me to close this @m8pple ?

heliosfa commented 3 years ago

Closing as I believe this has been resolved by docs changes