Skip to content

Conversation

alexcekay
Copy link
Member

Solved Problem

The IST8310 startup is currently a bit unexpected

  • A lot of boards start it up with -R 10 due to the way it is mounted in most GPS modules
  • It is only started on one bus in the rc.board_sensors
  • When connected to another bus it will fall-back to the start up in rc.sensors which does start on all buses, but with -R 0

Solution

  • Unified the startup by starting it up with -R 10 on all buses in rc.sensors
  • Kept it in rc.board_sensors for boards that start it with different rotation

Changelog Entry

For release notes:

Feature/Bugfix unify ist8310 startup

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

  • Tested on v6x with a Holybro GPS

@dagar
Copy link
Member

dagar commented Jul 2, 2025

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:
#15120

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
Copy link
Member

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.

@alexcekay
Copy link
Member Author

alexcekay commented Jul 2, 2025

Hi @dagar,
thanks for the input, I feared that this will be the case, this is why its still drafted.

One alternative:

  • We could autostart it with -R 10 for all buses for those boards that currently already start it with -R 10 in the board specific sensors file. Because currently when you plug in the GPS in another port you will get the unexpected behavior of the GPS starting with one rotation on one I2C port and another one on a different I2C port.
  • I assume the logic here is to just do the rotation for the ones in the GPS port, but when we already do rotation on one port it would be more consistent to apply this rotation independent of the port its plugged in.

FYI:
@sfuhrer, @MaEtUgR

@dagar
Copy link
Member

dagar commented Jul 2, 2025

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...

@alexcekay
Copy link
Member Author

Is there a particular case you want to make less bad? Things we might be able to autodetect or have presets.

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 -R 10, you will get a -R 10 independent of the bus you connect it to.

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:

ist8310 -X -R 10 start

instead of

ist8310 -X -b 1 -R 10 start

as can be seen in the adapted commit.

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

Interesting idea, however the rotation is currently encoded in CAL_MAGn_ROT, which only has semantic after calibration. This way we don't have the option to set the "fixed" rotation on a per magnetometer basis. So I only the the option of having a "per magnetometer" parameter, which adds another layer.

@sfuhrer
Copy link
Contributor

sfuhrer commented Jul 4, 2025

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".
However the outcome here, would be good to add a few lines to the user guide in the compass orientation section. There's no way for the average user to notice the -R in the startup.

@github-actions github-actions bot added the stale label Aug 4, 2025
@mrpollo mrpollo removed the stale label Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants