Redefine Simulator::handle_message() prototype and create Simulator::set_publish() method. #11564
Conversation
mcsauder
changed the title
mcsauder
changed the title
mcsauder
force-pushed
the
simulator_mavlink_handle_methods
branch
from
7d355b4 to
8391f77
Compare
| @@ -838,14 +837,17 @@ void Simulator::poll_for_MAVLink_messages(bool publish) | |||
| for (int i = 0; i < len; ++i) { | |||
| if (mavlink_parse_char(MAVLINK_COMM_1, serial_buf[i], &msg, &serial_status)) { | |||
| // have a message, handle it | |||
| handle_message(&msg, true); | |||
| set_publish(true); | |||
| handle_message(&msg); | |||
There was a problem hiding this comment.
You should do it like this:
bool prev_publish = _publish;
_publish = true;
handle_message(&msg);
_publish = prev_publish;
Otherwise you're affecting and overriding the state for the next loop iteration.
There was a problem hiding this comment.
Thanks @bkueng ,
Changes requested are pushed up!
Otherwise you're affecting and overriding the state for the next loop iteration.
You are correct, that was intentional. The current logic in master cannot allow a persistent publish == true state to exist outside of one loop. The set_publish(false) call added at the end of the loop was a guard against that new possibility. With the latest commit that guard has been removed.
Let me know if you spot anything else I can address. Thanks for your feedback!
mcsauder
force-pushed
the
simulator_mavlink_handle_methods
branch
4 times, most recently
from
a584ed9 to
ff82a7f
Compare
… variable to allow redefinition of the Simulator::handle_message() prototype to match MavlinkReceiver::handle_message().
…)set each loop in simulator_mapvlink.cpp and provide a default argument to set_publish() in simulator.h.
mcsauder
force-pushed
the
simulator_mavlink_handle_methods
branch
from
ff82a7f to
79cab05
Compare
src/modules/simulator/simulator.h
Outdated
| void request_hil_state_quaternion(); | ||
| void send(); | ||
| void send_controls(); | ||
| void send_heartbeat(); | ||
| void send_mavlink_message(const mavlink_message_t &aMsg); | ||
| void set_publish(const bool publish = false); |
There was a problem hiding this comment.
This default is confusing to me: if I were to see a simulator->set_publish(), I would not expect it to disable publication.
There was a problem hiding this comment.
No worries, I can set it to default true.
|
Awesome! Thanks! |
Describe problem solved by the proposed pull request
This PR redefines the
Simulator::handle_message()prototype to matchMavlinkReceiver::handle_message()and creates theSimulator::set_publish()method and associated_publishmember variable to preserve functionality.The work in this PR creates the path for additional refactoring of the Simulator::handle_message() started in #11499.
Please let me know if you have any questions about this PR. Thanks!
-Mark
@bkueng