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

Let user choose which EK3_SRC params to change #41

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

clydemcqueen
Copy link
Contributor

Do not set EK3_SRC parameters by default, but provide buttons instead.

2 use cases are supported:

  • use the DVL for horizontal position and velocity
  • use the GPS for horizontal position, and the DVL for horizontal velocity

Fixes #35

@clydemcqueen clydemcqueen marked this pull request as ready for review June 29, 2024 22:35
@clydemcqueen
Copy link
Contributor Author

Updated to pass the pylint CI (I hope).

I don't know why the deploy CI is failing. I marked the PR 'ready for review' to see if that helps.

@Williangalvani
Copy link
Member

Williangalvani commented Jul 1, 2024

Oh, the deploy CI is apparently wrong, it should not try to run on PRs.

@clydemcqueen don't we need EK3_SRC_OPTIONS for fusing? I see it set to "4" on your issue but that doesn't seem valid

@clydemcqueen
Copy link
Contributor Author

don't we need EK3_SRC_OPTIONS for fusing? I see it set to "4" on your issue but that doesn't seem valid

"4" is type; the value is 1. (That's how QGC dumps parameters.)

In my earlier example I had this:

  • SRC1_POSXY GPS
  • SRC1_VELXY GPS
  • SRC2_VELXY DVL
  • SRC_OPTIONS 1

I did more simulations after that, and this worked just as well:

  • SRC1_POSXY GPS
  • SRC1_VELXY DVL
  • SRC_OPTIONS is not used

I've been using this simpler setup in the very limited IRL tests we've done so far, and it works just fine. Perhaps more extensive tests will prove that one method works better than the other? Of a careful tracing of the EKF code might prove that they are equivalent...

@Williangalvani
Copy link
Member

"4" is type; the value is 1. (That's how QGC dumps parameters.)

lol I misread that badly, sorry. I'll try to test this asap, thanks!

Copy link
Member

@Williangalvani Williangalvani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested and works as expected. user feedback would be nice, but I think the feature is worth merging asap.

Thanks Clyde!

@Williangalvani Williangalvani merged commit ce1b939 into bluerobotics:master Jul 11, 2024
0 of 2 checks passed
@clydemcqueen clydemcqueen deleted the clyde_fix_35 branch July 11, 2024 14:56
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.

Fusing UGPS and DVL is difficult (extension overwrites EK3_SRC1_POSXY and EK3_SRC1_VELXY)
2 participants