Skip to content

Act on compass read return value#511

Merged
digitalentity merged 3 commits intoiNavFlight:masterfrom
martinbudden:inav_compass_read
Aug 28, 2016
Merged

Act on compass read return value#511
digitalentity merged 3 commits intoiNavFlight:masterfrom
martinbudden:inav_compass_read

Conversation

@martinbudden
Copy link
Copy Markdown
Contributor

Stops ignoring the return value of the compass read function. Takes action if read returns false.

@stronnag
Copy link
Copy Markdown
Collaborator

Very interesting. I've just rebuilt the quad with the dodo to eliminate the GPS/compass, so it'll probably be Monday before I try this.

@digitalentity
Copy link
Copy Markdown
Member

digitalentity commented Aug 27, 2016 via email

mag.read(magADCRaw);
for (axis = 0; axis < XYZ_AXIS_COUNT; axis++) magADC[axis] = magADCRaw[axis]; // int32_t copy to work with
if (!mag.read(magADCRaw)) {
return;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We still need to return zero vector here so IMU can detect invalid compass and act accordingly.

@martinbudden
Copy link
Copy Markdown
Contributor Author

I've updated to return a zero value in sensors/compass.c if the mag.read fails.

One other thought: in initialisation it tries to collect 10 samples, if any read fails there will be less than ten samples. Should I change it to collect 10 valid reads? And put in some protection so that if the reads continually fail it does not get stuck in the loop?

@digitalentity
Copy link
Copy Markdown
Member

digitalentity commented Aug 28, 2016 via email

@martinbudden
Copy link
Copy Markdown
Contributor Author

OK, I've improved the handling of invalid readings in initialisation.

The init function is void not bool - I think that should be changed as well, but that is more pervasive, so I'll wait until @stronnag has tested this before I make that change.

@digitalentity
Copy link
Copy Markdown
Member

I agree to both:

  • we need to test and merge this first
  • init function should be able to fail as well and the result of failure should be a non-present sensor indication.

@digitalentity
Copy link
Copy Markdown
Member

I think it's safe to merge this right away. @martinbudden is this ready to merge?

@digitalentity
Copy link
Copy Markdown
Member

See #514 for further discussion.

@martinbudden
Copy link
Copy Markdown
Contributor Author

Yes, you can merge this now.

@digitalentity digitalentity merged commit de24cd0 into iNavFlight:master Aug 28, 2016
@martinbudden martinbudden deleted the inav_compass_read branch August 29, 2016 10:15
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.

3 participants