Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#3427 closed defect (fixed)

CPP runtime does not handle OMEdit simFlags correctly

Reported by: Marcus Walther Owned by: Marcus Walther
Priority: high Milestone: 1.9.4
Component: *unknown* Version: trunk
Keywords: Cc: Rüdiger Franke, adrpro, Niklas Worschech

Description

In #3426 was reported that the following error occurs if the MSL model "Modelica.Blocks.Examples.PID_Controller" is simulated with OMEdit and +simCodeTarget=cpp.

stdout | error | <p>Warning: unrecognized command line options -port=8792 -logFormat=xml -override=variableFilter=.* -dasslJacobian=coloredNumerical -lv=LOG_STATS -w

We need a list of flags that are used by OMEdit, even if we do not support the features related to the flag. Furthermore we need a code-refactoring, because the flag "emit_protected" is part of OMCFactory::readSimulationParameter but not supported by the runtime. These OMEdit-flags should be fully separated from the supported flags of the runtime. Ruediger has already implemented functions to solve that, but they are not used for all flags. Furthermore a flag-test should be added to the testsuite.

Change History (12)

comment:1 by Marcus Walther, 9 years ago

Owner: set to Marcus Walther
Status: newaccepted

I've changed the code in OMCFactory slightly. We can now ignore flags and we do not print a warning, if C-Runtime flags are detected. I'm not sure if this is a good idea, but it will prevent the warning in OMEdit.
@rfranke What do you think about that? Is it important to print the warning for the OMEdit-flags?

Besides that I have added a test to testsuite/openmodelica/cppruntime/omedit to check if we can simulate correctly, even if flags in c-runtime format are passed to the executable.

comment:2 by Rüdiger Franke, 9 years ago

I don't like that change. The intention of the warning is to make clear which options have been ignored in case someone wonders -- and as a reminder to eventually implement those options. -port for instance feeds the progress bar. Now, if you wonder why the progress bar doesn't work, you don't even get a warning anymore that this is not (yet) supported by the Cpp runtime.

Could you revert that change?

The test is a good idea -- it should always contain an up-to-date options string from OMEdit.

comment:3 by Marcus Walther, 9 years ago

I agree, it's strange to supress the warning, but because I'm not using OMEdit, I would give the decision about this to you. Reverting the change is really easy, I will do this on monday.
But I think we should keep the ignore-list for some arguments. For example emit_protected is always enabled in the c++ runtime, so there must not be a warning if the flag is set.

Version 0, edited 9 years ago by Marcus Walther (next)

comment:4 by Marcus Walther, 9 years ago

I've reverted my changes, the warning should now appear as expected. See b6bb89dfa4568cd7c412398441a394338dd0048b for details. The documentation update will follow.

@Niklas and @Ruediger: There is one thing left. We still have the argument "-r" - the only argument written with a lower cases letter. Can I rename this argument into a capital letter (e.g. -F) and add a mapping from "-r" to "-F". Than we have a clean separation between the c and c++ runtime arguments.

comment:5 by Niklas Worschech, 9 years ago

From my side you can do this.

comment:6 by Rüdiger Franke, 9 years ago

The -r option is small because it is compatible with the C runtime. No need to rename it.

comment:7 by Marcus Walther, 9 years ago

I think it's bad smelling code if we use just capital letter arguments, but the -r is small. It should be consistent.

comment:8 by Rüdiger Franke, 9 years ago

Fine. Just don't forget to also support "-r", as it is used by OMEdit by default.

comment:9 by Marcus Walther, 9 years ago

We can now overwrite flags. At the moment we are replacing "-w" with "-V all=warning" and "-r=path" with "-F=path". This leads to the fact, that we can have 2 different values for the same argument. For example "model -F regularCPPResultFile.mat -F=resultFileOfOMEdit.mat". To support that, I have changed the type of the -F flag from string to vector<string> and I pick the first value of the vector, which is always the OMEdit-value.

comment:10 by Marcus Walther, 9 years ago

Resolution: fixed
Status: acceptedclosed

comment:11 by Dietmar Winkler, 9 years ago

Milestone: Futurepre1.9.4

It doesn't make sense to keep closed ticket in the "Future" milestone that were simply forgotten to assign to the correct milestone in the past.

comment:12 by Martin Sjölund, 7 years ago

Milestone: pre1.9.41.9.4

Removing the pre1.9.4 milestone in favor of 1.9.4.

Note: See TracTickets for help on using tickets.