-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ESmini simulator #223
Add ESmini simulator #223
Conversation
11d76de
to
593a8f9
Compare
30aec1e
to
0ad5fc9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I am placing this PR, I cannot approve. But I did review all the code from Tobias and fixed several findings I had. In all, looks good!
da6de28
to
358d004
Compare
1d40600
to
d6edb3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
osi_transceiver_tcp needs to be fixed but for for EsMini it does not matter currently --> osi_transceiver_tcp may be fixed in the future!
std::vector<std::shared_ptr<osi3::SensorData>> receive_sensor_data() override { | ||
std::vector<std::shared_ptr<osi3::SensorData>> msgs; | ||
bool has_sensor_view() const override { | ||
return this->tcp_available_data() >= static_cast<std::streamsize>(sizeof(osi3::SensorView)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isnt sizeof(osi3::SensorView) the size of the deserialized protobuf message whereas the size of the message in the tcp buffer is serialized?
In my point of view the amount of data in the buffer does not give any information on which osi message is in the buffer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this does look sketchy.
The optional directory contains projects that are hard to build. ESMini does not fall into this category, since the sources are freely available. This commit does the following: - Move optional/esmini to plugins/esmini - Move optional/osi to osi - Replace Conan recipe for vendor/esmini and vendor/esmini-data - Replace Conan recipe for vendor/open-simulation-interface - Patch the esmini and osi Cloe libraries for compatibility with new open-simulation-interface. In particular, the generated headers are now namespaced with osi3/ - Fix several findings in esmini and osi Cloe libraries highlighted by clang-tidy. The vendored libraries are written in a manner similar to how it is done in the Conan-Center-Index.
d6edb3d
to
375afa8
Compare
Still needs to be rebased, but otherwise ready for review.