-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
simulator move to PX4Accelerometer, PX4Gyroscope, PX4Magnetometer, PX4Barometer helpers #12081
Conversation
f3a9495
to
5b7bf4a
Compare
The tiltrotor SITL failure is real, but I think it's more along the lines of not letting something sketchy in that gazebo model slip through anymore with this change. |
Let's disable tiltrotor for now until someone can take a closer look. It's failing quite often in other PRs and master as well. #12085 |
@dagar does the CI fail happen only with these changes or in general ? |
5b7bf4a
to
1a2cdcf
Compare
In general. PR rebased on master (with tiltrotor temporarily disabled). |
@@ -166,11 +126,6 @@ int Simulator::start(int argc, char *argv[]) | |||
_instance->poll_for_MAVLink_messages(); | |||
#endif | |||
|
|||
} else if (argv[2][1] == 'p') { |
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.
I think this breaks HITL for Raspberry Pi or something like that but I'm not sure anymore.
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.
Shouldn't matter anymore without the need for most of the simulated sensors.
The problem with this change is that we can no longer simulate sensor failover which is something we should do. However, if this change means it's easier to refactor sensor drivers and to remove the DriverFramework, then I'm ok with it for now. |
Yes I'm not happy about that part either, but realistically we need to do a lot more than simulate the data stopping. |
36aa412
to
78b659d
Compare
78b659d
to
9a28966
Compare
…4Barometer helpers
9a28966
to
8c3e73a
Compare
The sim drivers are a good idea, but the implementation is a bit of a mess and they're getting in the way of larger sensor pipeline changes and dropping DriverFramework. For now I propose dropping them and simply using the helper classes (PX4Accelerometer, PX4Gyroscope, etc) directly in the simulator.
Not only is this much simpler, but it keeps the details in sync with what the real drivers are actually doing (filting, integration, respecting parameters IMU_GYRO_CUTOFF, etc).
Longer term I'd like to resurrect the sim driver idea, but instead do it as a simulation interface to a real driver.