New branch of visualizer for osi-2.2.1 maintenance #20
Conversation
ghost
requested a review
from 
There was a problem hiding this comment.
The FMU access should always be mediated through the variable names, since that is the only thing that is specified to be well-defined. VRs are internal to the FMU, the mapping name -> vr must be looked up upon loading the FMU. For the non-OSMP specified parameters (like sender, receiver, ...), setting those should be under the control of the user (hard-coding for now should be done on compiling the FMU, it seems to me).
src/fmureceiver.cpp
Outdated
| return; | ||
| } | ||
|
|
||
| if (fmi2_import_get_fmu_kind(fmu_) == fmi2_fmu_kind_me) |
There was a problem hiding this comment.
Test should follow logic in warning message, i.e. test for != fmi2_fmu_kind_cs, rather than me.
src/fmureceiver.cpp
Outdated
| // start values | ||
| fmi2_string_t instanceName = "Test CS model instance"; | ||
|
|
||
| fmi2_string_t fmuLocation = ""; |
There was a problem hiding this comment.
fmuLocation should be changed to be FMI compliant (i.e. needs to point to resource directory location of unpacked FMU).
src/fmureceiver.cpp
Outdated
| fmiStatus_ = fmi2_import_enter_initialization_mode(fmu_); | ||
| fmiStatus_ = fmi2_import_exit_initialization_mode(fmu_); | ||
|
|
||
| vr_[FMI_INTEGER_SENSORDATA_IN_BASELO_IDX] = FMI_INTEGER_SENSORDATA_IN_BASELO_IDX; |
There was a problem hiding this comment.
The variable accesses should be done via the specified variable names, not through hard-coded VRs (these can change at any time, only the Variable names are guaranteed to remain constant through the OSMP spec). E.g. variables can be looked up via fmi2_import_get_variable_by_name and their VR via fmi2_import_get_variable_vr, all to be done at load-time of the FMU.
src/fmureceiver.cpp
Outdated
| // #define FMI_BOOLEAN_DUMMY_IDX 0 | ||
| // #define FMI_BOOLEAN_SENDER_IDX 1 | ||
| // #define FMI_BOOLEAN_RECEIVER_IDX 2 | ||
| fmi2_boolean_t booleanVars_[3]; |
There was a problem hiding this comment.
Dito for the booleans, but here OSMP does not guarantee that they exist at all, so this should be user configurable...
src/fmureceiver.cpp
Outdated
| bool | ||
| FMUReceiver::get_fmi_sensor_data_in(osi::SensorData& data) | ||
| { | ||
| fmiStatus_ = fmi2_import_get_integer(fmu_, vr_, FMI_INTEGER_VARS, integerVars_); |
There was a problem hiding this comment.
This also gets the inputs, which does not seem to make sense for the code base here; since inputs are not set anywhere here, they should not be accessed at all, it seems to me.
src/osireader.cpp
Outdated
| if (!fmu_ && errMsg.isEmpty()) | ||
| errMsg = "Error parsing modeldescription.xml of fmu "; | ||
|
|
||
| if (fmi2_import_get_fmu_kind(fmu_) == fmi2_fmu_kind_me && errMsg.isEmpty()) |
There was a problem hiding this comment.
Same comments as for fmu receiver apply.
src/osireader.cpp
Outdated
| integerVars_[FMI_INTEGER_SENSORDATA_IN_BASELO_IDX]); | ||
| integerVars_[FMI_INTEGER_SENSORDATA_IN_SIZE_IDX] = (fmi2_integer_t)currentBuffer_.length(); | ||
|
|
||
| fmiStatus_ = fmi2_import_set_integer(fmu_, vr_, FMI_INTEGER_VARS, integerVars_); |
There was a problem hiding this comment.
Same comments as for the receiver: The outputs should not be set here at all.
…ive message time stamp; add scroll bar for the config info.
|
@haoyuanying this needs to be fixed for the maintenance branch |
ghost
added
the
feature request
No description provided.