Reset overrides from outside #206
Conversation
…n step commences.
There was a problem hiding this comment.
Some of the code lines introduced in this PR are very long, and won't even fit in the diff view on GitHub. I think we decided at some point to have a soft limit of 80 characters per line, and a hard limit of 100. We just haven't been able to get clang-format to enforce it in any good way. I'd appreciate it if the long lines were broken up a bit.
Otherwise very good, just some minor issues that need to be addressed. :)
src/cpp/override_manipulator.cpp
Outdated
| @@ -1,3 +1,5 @@ | |||
| #include "../../include/cse/scenario.hpp" | |||
There was a problem hiding this comment.
Why not #include "cse/scenario.hpp"?
There was a problem hiding this comment.
CLion works in mysterious ways, but I should have caught it! I'll remove it, as scenario.hpp is already included in manipulator.hpp.
src/cpp/override_manipulator.cpp
Outdated
| } | ||
|
|
||
| void override_manipulator::override_real_variable(simulator_index index, variable_index variable, double value) | ||
|
|
||
| template<typename I, typename O, typename M, typename N> |
There was a problem hiding this comment.
With so many template parameters it is no longer obvious what they mean. I suggest some more descriptive names, or at least an explanatory comment.
There was a problem hiding this comment.
I agree! I also don't really like how this turned out, so I'm working on a cleaner way to do it.
…lator. Directionality is stored in a flag in variable_action.
I have tried reducing line lengths in the files I've touched now. Other than that, I've pushed some changes restructuring the whole shebang with the Opinions? |
This solves part of issue #190, allowing resetting of overridden values from the C API.
As a safeguard against potential race conditions,
cse::override_manipulatornow only callscse::simulator::set_xxx_yyy_manipulator()during the call tocse::manipulator::step_commencing(). I may have taken things a bit too far with templates here, as well as borrowing some code from the scenario world in order to minimize code duplication.One obvious downside to this feature, is that when resetting parameters and unconnected inputs, the value goes back to 0, as mentioned in issue #188. I suggest that this be fixed in a separate PR, so as to not clash with PR #189.
For testing the client-server-core trifecta, a distribution can be downloaded from the related cse-server-go build.