-
Notifications
You must be signed in to change notification settings - Fork 14.6k
ist8310: unify startup #25145
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
base: main
Are you sure you want to change the base?
ist8310: unify startup #25145
Conversation
Unfortunately there are a lot of mags out there that have the ist8310 mounted differently than 10 (roll 180, yaw 90). This is why I implemented the crude mag auto rotation during calibration and got rid of the "common" sensor rotation that was originally in the driver itself. Background: So we can't really win, but we cheated a bit for setups that typically come with a known mag in a kit like the Holybro v6x with Holybro GPS/mag puck (ist8310 at R 10) in the "GPS" port. |
|
||
# standard Holybro Pixhawk 4 or CUAV V5 GPS/compass puck (with lights, safety button, and buzzer) | ||
# if the ist8310 has a different rotation, start it in rc.board_sensors | ||
ist8310 -X -R 10 -q start |
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.
This will break existing setups.
Hi @dagar, One alternative:
|
Is there a particular case you want to make less bad? Things we might be able to autodetect or have presets. If anything we should try to minimize starting external sensors with any -R and try to capture these initial guesses in parameter configuration (currently CAL_MAGn_ROT). Ultimately if things get really confusing people should be able to resort to checking the actual chip orientation for sanity. I'm not sure if I even like this idea, but the bulk of the difference falls into -R 10 (Holybro, CUAV, and a few others) vs what mRo did. If we moved the initial guess rotation to a parameter default per board I suspect we could get closer to being right more of the time, but annoyingly we still can't fix this 100%. It really shouldn't have been that hard to do this right in the first place, but here we are... |
What I want is to have a consistent handling of Holybro GPS. Basically: Whenever you connect any Holybro GPS to any of the boards that already have This is more consistent: "Whenever you connect a Holybro GPS to any of those boards you will get a fixed rotation" instead of "When you connect a Holybro GPS to any of those boards you will get a fixed rotation, but just on one bus". So basically:
instead of
as can be seen in the adapted commit.
Interesting idea, however the rotation is currently encoded in |
I tentatively agree with @alexcekay on removing the bus specification in the board_sensors. Though it then means that for these boards, any plugged ist8310 will be started with the rotation. @dagar you're the best to judge how many folks are using the ist8310 without rotation, from what you wrote above you seem to agree that it's not so many "the bulk of the difference falls into -R 10 (Holybro, CUAV, and a few others) vs what mRo did". |
Solved Problem
The IST8310 startup is currently a bit unexpected
-R 10
due to the way it is mounted in most GPS modulesrc.board_sensors
rc.sensors
which does start on all buses, but with-R 0
Solution
-R 10
on all buses inrc.sensors
rc.board_sensors
for boards that start it with different rotationChangelog Entry
For release notes:
Alternatives
We could also start it up on all buses in
rc.board_sensors
, but as a lot of boards do start it up with-R 10
it is easier to have a unified behavior.Test coverage