Add new FMU node-type - #1017
Conversation
Signed-off-by: Ritesh.K <riteshkarki6@gmail.com> Signed-off-by: Alexandra <alexandra.bach@eonerc.rwth-aachen.de>
Signed-off-by: Ritesh.K <riteshkarki6@gmail.com> Signed-off-by: Alexandra <alexandra.bach@eonerc.rwth-aachen.de>
Signed-off-by: Ritesh.K <riteshkarki6@gmail.com> Signed-off-by: Alexandra <alexandra.bach@eonerc.rwth-aachen.de>
Signed-off-by: Ritesh.K <riteshkarki6@gmail.com> Signed-off-by: Alexandra <alexandra.bach@eonerc.rwth-aachen.de>
Signed-off-by: Ritesh.K <riteshkarki6@gmail.com> Signed-off-by: Alexandra <alexandra.bach@eonerc.rwth-aachen.de>
Signed-off-by: Alexandra <alexandra.bach@eonerc.rwth-aachen.de>
Signed-off-by: Alexandra <alexandra.bach@eonerc.rwth-aachen.de>
Signed-off-by: Ritesh.K <riteshkarki6@gmail.com>
stv0g
left a comment
There was a problem hiding this comment.
Nice work. Its great to see FMI support landing in VILLAS
| while (hasInputs && (writingTurn || currentTime >= stopTime)) { | ||
| pthread_cond_wait(&cv, &mutex); | ||
| } | ||
| double targetTime = currentTime + stepSize; |
There was a problem hiding this comment.
I think this not executing the FMU using a real-time scale?
Can you elaborate a bit how this node-type is indentend to be used?
E.g. does it expect periodic input data to proceed its internal time?
How does the internal time related to the real time?
Do we already have a PR for the node-type? I think this deserves some explanation.
There was a problem hiding this comment.
Right now, it needs periodic input data. My plan would be to include real-time later
Signed-off-by: Alexandra <alexandra.bach@eonerc.rwth-aachen.de>
pjungkamp
left a comment
There was a problem hiding this comment.
I really like the general coding style, it's clean and very readable. Could you please fix the commit such that you have one commit that introduces the FMU node type. You have two unrelated changes changing the capitalization of writing_turn in opendss.{hpp,cpp} which should not be part of that commit. You can either move those changes into their commit or drop them.
| description: | | ||
| Specifies the URI to a directory which has the unpacked files of the FMU model (mainly the shared library and the modelDescription.xml for the model). | ||
|
|
||
| fmuWritefirst: |
There was a problem hiding this comment.
| fmuWritefirst: | |
| fmu_write_first: |
| description: | | ||
| Specifies whether the model has to be writtem with input values before running. `true` denotes that the model should use the input values. `false` would make the model use default values instead. | ||
|
|
||
| startTime: |
There was a problem hiding this comment.
| startTime: | |
| start_time: |
| description: | | ||
| Specifies the start time for the simulation. Value has to be at least one `stepSize` lesser than `stopTime`. | ||
|
|
||
| stopTime: |
There was a problem hiding this comment.
| stopTime: | |
| stop_time: |
| description: | | ||
| Specifies the stop time for the simulation. The number of steps is calculated then by the difference between `startTime` and `stopTime` divided by the `stepSize`. | ||
|
|
||
| stepSize: |
There was a problem hiding this comment.
| stepSize: | |
| step_size: |
| file_output = { | ||
| type = "file" | ||
| # format = "csv" | ||
| uri = "/workspaces/node/fmu_temp/output.dat" | ||
| }, | ||
|
|
||
| signal_node = { | ||
| type = "signal.v2" | ||
| rate = 100.0 | ||
| realtime = true | ||
|
|
||
| limit = 100 | ||
|
|
||
| in = { | ||
| signals = ( | ||
| { name = "sine1", signal = "sine", amplitude = 1, frequency = 100 }, | ||
| # { name = "in2", signal = "constant", amplitude = 1 }, | ||
| ) | ||
| } | ||
| }, |
There was a problem hiding this comment.
| file_output = { | |
| type = "file" | |
| # format = "csv" | |
| uri = "/workspaces/node/fmu_temp/output.dat" | |
| }, | |
| signal_node = { | |
| type = "signal.v2" | |
| rate = 100.0 | |
| realtime = true | |
| limit = 100 | |
| in = { | |
| signals = ( | |
| { name = "sine1", signal = "sine", amplitude = 1, frequency = 100 }, | |
| # { name = "in2", signal = "constant", amplitude = 1 }, | |
| ) | |
| } | |
| }, |
The example configuration file is embedded into the documentation and should only contain the node that this example is for. Runnable examples aren't part of this repo.
|
|
||
| public: | ||
| FmuNode(const uuid_t &id = {}, const std::string &name = ""); | ||
| struct fmu_signal { |
There was a problem hiding this comment.
Types/classes defined in villas-node are PascalCase. Rename fmu_signal to FmuSignal and move it outside the FmuNode.
| ret = json_unpack_ex( | ||
| json, &err, 0, "{s:s, s:s, s?:f, s?:f, s?:f, s?:{s:o}, s?:{s:o}}", | ||
| "fmu_path", &path, "fmu_unpack_path", &unpack_path, "stopTime", &stopTime, | ||
| "startTime", &startTime, "stepSize", &stepSize, "in", "signals", | ||
| &json_signals_in, "out", "signals", &json_signals_out); |
There was a problem hiding this comment.
| ret = json_unpack_ex( | |
| json, &err, 0, "{s:s, s:s, s?:f, s?:f, s?:f, s?:{s:o}, s?:{s:o}}", | |
| "fmu_path", &path, "fmu_unpack_path", &unpack_path, "stopTime", &stopTime, | |
| "startTime", &startTime, "stepSize", &stepSize, "in", "signals", | |
| &json_signals_in, "out", "signals", &json_signals_out); | |
| ret = json_unpack_ex( | |
| json, &err, 0, "{s:s, s:s, s?:f, s?:f, s?:f, s?:{s:o}, s?:{s:o}}", | |
| "fmu_path", &path, "fmu_unpack_path", &unpack_path, "stop_time", &stopTime, | |
| "start_time", &startTime, "step_size", &stepSize, "in", "signals", | |
| &json_signals_in, "out", "signals", &json_signals_out); |
| } | ||
| logger->debug("Input value {}", val); | ||
| switch (signalIn[i].type) { | ||
| case fmi3_base_type_enu_t::fmi3_base_type_float64: |
There was a problem hiding this comment.
| case fmi3_base_type_enu_t::fmi3_base_type_float64: | |
| using enum fmi3_base_type_enu_t; | |
| case fmi3_base_type_float64: |
This is just a suggestion. You can introduce a using enum statement before the first case statement to make the cases a little more readable.
| } | ||
|
|
||
| parse_signal(json_signals_in, true); | ||
| parse_signal(json_signals_out, false); |
There was a problem hiding this comment.
It seems that you only need the name from the in.signal/out.signals. That name is already parsed as part of Node::parse above. You can get the names by iterating the signal lists from getInputSignals or getOutputSignals, which would be the more idiomatic approach here.
| static NodePlugin<FmuNode, n, d, | ||
| (int)NodeFactory::Flags::SUPPORTS_READ | | ||
| (int)NodeFactory::Flags::SUPPORTS_WRITE | | ||
| (int)NodeFactory::Flags::SUPPORTS_POLL> |
There was a problem hiding this comment.
Does your node support poll? Seems to me like it doesn't.
@al3xa23
New node type implementation based on Functional Mock-up Interface (FMI 3.0). Currently supports Co-Simulation. Functionality can be further extended to include Model Exchange simulation too.