Skip to content
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

Skip antenna height correction when IMU is active #4503

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

domi4484
Copy link
Member

@domi4484 domi4484 commented Aug 9, 2023

When IMU is active, the GNSS device provides the position at the end of the pole.
The antenna height must not be subtracted again from the Z value.

@qfield-fairy
Copy link
Collaborator

qfield-fairy commented Aug 9, 2023

@mbernasocchi mbernasocchi requested a review from nirvn August 9, 2023 13:47
@nirvn
Copy link
Member

nirvn commented Aug 10, 2023

@domi4484 , I was re-thinking about this earlier today, and I don't think we can apply the antenna height at the receiver level, as it'll likely mess up (infinitely small value but still) vertical grid shift calculation here.

We ultimately need to add the antenna height value after we transformed the QgsPoint( lat, lon, elevation ) to the desired destination CRS. So, I think we're back to handling this in the Positioning class, and insure that the destination point's Z value has the antenna height added after the transformation.

@domi4484
Copy link
Member Author

We ultimately need to add the antenna height value after we transformed the QgsPoint( lat, lon, elevation ) to the desired destination CRS. So, I think we're back to handling this in the Positioning class, and insure that the destination point's Z value has the antenna height added after the transformation.

I see only a small problematic with that approach:

  • No IMU active: QField does the antenna correction AFTER crs transformation
  • IMU active: Antenna correction is done by receiver BEFORE crs transformation

Well if the difference is in infinitely small range maybe better keep it consistent between modes?

@nirvn
Copy link
Member

nirvn commented Aug 11, 2023

@domi4484 , fair point.

@domi4484 domi4484 closed this Aug 14, 2023
@domi4484 domi4484 reopened this Aug 14, 2023
@domi4484
Copy link
Member Author

@nirvn I tried to restart the test but the ios is still failing when downloading qt... Can I merge anyway?

@nirvn
Copy link
Member

nirvn commented Aug 14, 2023

@domi4484 , yeah go ahead

@domi4484
Copy link
Member Author

@domi4484 , yeah go ahead

Apparently I can't skip merge protections...

@mbernasocchi mbernasocchi enabled auto-merge (squash) August 15, 2023 19:37
@mbernasocchi mbernasocchi merged commit a8606c8 into master Aug 15, 2023
38 of 40 checks passed
@mbernasocchi mbernasocchi deleted the ignoreAntennaHeightOnIMUwq branch August 15, 2023 19:58
@mbernasocchi
Copy link
Member

@domi4484 , yeah go ahead

Merging even if iOS did not build, QT download still fails

nirvn pushed a commit that referenced this pull request Aug 19, 2023
* Skip antenna height correction when IMU is active

* Move antenna height logic to the receiver
nirvn pushed a commit that referenced this pull request Aug 20, 2023
* Skip antenna height correction when IMU is active

* Move antenna height logic to the receiver
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.

None yet

4 participants