The logger callback function should forward the fmi2ComponentEnvironm…

[zefir-o]

…ent that was passed during the FMU instantiation. This allows the simulation environment to provide necessary context or state information to the logger.

[zefir-o]

@jschueller @jrbull @markaren @fcollonval please review the PR

[jschueller]

I'm not relevant to review this

[zefir-o]

Hi @markaren, could you please take a look into the PR?

[markaren]

I have limited time to look at this project, but as far as I can see what is changed here is that the type is changed from Component* (which could hold the componentEnvironment) to just the pointer to componentEnvironent).

What about rather exposing callbackFunctions.componentEnvironment through the existing pointer? Unsure what to to in the failure state.

[zefir-o]

Hi @markaren , thank you for your time. Yep, you are right, instead of passing a Component as the fmi2ComponentEnvironment in Logger callback, the user-defined callbackFunctions.componentEnvironment is forwarded to the logger callback always. The FMI standard requires such a logic. Basically, there was a bug, as it's not allowed to pass to the logger just some random pointer (which was Component) previously.

[Nils12345678901234567]

This is a major bug. If the environment that implements the logging method tries to access the pointer it will crash.

The library https://github.com/rwth-irt/FmiWrapper does exactly this an will crash if logging is enabled:

/*!
Implementation of the logger callback that is passed to the fmu.
This function calls the logCallback of the wrapper.
*/
static void fmuLogCallback(fmi2ComponentEnvironment component_environment, fmi2String instance_name, fmi2Status status, fmi2String category, fmi2String message, ...)
{
    wrapped_fmu *wrapper = (wrapped_fmu*)component_environment;
    // fmi2standard: The message is to be used like sprintf.
    // For simplification apply the variadic arguments to the format string and call enviromentLog with this single string
    va_list args;
    va_start(args, message);
    // Get the size of the string + 1 for the \0 char.
    int needed_size = vsnprintf(NULL, 0, message, args) + 1;
    // Create and read into buffer with size + 1 (for \0 character)
    char *buffer = malloc(needed_size);
    vsnprintf(buffer, needed_size, message, args);
    wrapper->log(instance_name, status, category, buffer);

(https://github.com/rwth-irt/FmiWrapper/blob/master/src/c_wrapper/fmi_wrapper.c)

[markaren]

@Nils12345678901234567 What do you propose, and what is the underlying issue here?

[zefir-o]

I looked into PR https://github.com/rwth-irt/FmiWrapper/issues/9 which @Nils12345678901234567 submitted, and it's mentioned, that there is a bug in the current release of PythonFMU. However, this particular PR is not yet part of any release. I also think this PR will fix the issue you mentioned by forwarding a componentEnvironment pointer via a logger callback.

Please, @Nils12345678901234567 build the master branch locally and check if the issue persists or not.

[Nils12345678901234567]

@markaren @zefir-o Yes, I think the PR fixes the problem. I added my comment because the current release has this issue and it will case a crash in some environments.

[zefir-o]

Adding a comment to a closed PR that something is not working can be misleading, as it suggests that this PR is causing the issue. In reality, this PR fixed the original problem.

Let me double-check:

• The latest existing release causes a crash.
• If you build the library locally from the master branch, there is no crash. Is that correct?

If so, and it's crucial for you let's ask @markaren to release a new version that includes the fix.