AP_Compass: Add in BMM350 Driver #28079
Conversation
|
I have just modified my pr base on your suggestions and still making a test... Any way, thank you for that! |
peterbarker
force-pushed
the
pr/bmm350
branch
from
497c782
to
08bd34c
Compare
|
Has to wait until #28088 goes in Fixed a problem with |
| if (temp > 0.0f) { | ||
| temp -= 25.49f; | ||
| } else if (temp < 0.0f) { | ||
| temp += 25.49f; | ||
| } |
There was a problem hiding this comment.
Ping @cuav-chen2 - do you see a problem with these lines?
There was a problem hiding this comment.
oh...my fault. We should compare with FLT_EPSILON here.
There was a problem hiding this comment.
I think there is something more serious. Does the offset really change depending on the temperature sign? Right now if the input temperature value changes from e.g. 0.001 to -0.001 the reported temperature will change by 50°C (in the opposite direction too!)
There was a problem hiding this comment.
Maybe you're right. I got this handle logic from the official routine of the sensor. I could do some testing on it if possible, but I'm not sure if I could control the temperature to this critical value.
There was a problem hiding this comment.
Have you checked the data sheet? This should definitely be in there.
Speaking of which, dropping a link to the datasheet in the comment at the top of the header is always a nice touch.
There was a problem hiding this comment.
The datasheet is particularly unhelpful, despite being 63 pages it doesn't have any description of the values and just refers to a broken code link...
I did do some searching and found the code and it does seem to be equivalent to what is proposed here, so I guess I can't argue any further.
Ardupilot does not log magnetometer temperature and Bosch's official API does not document the temperature units anyhow.
There was a problem hiding this comment.
OK, I'm marking this for merge - if it doesn't work we can fix it, nothing will crash because of this :-)
| // Store in field vector and convert uT to milligauss | ||
| Vector3f field { cr_ax_comp_x * 10.0f, cr_ax_comp_y * 10.0f, cr_ax_comp_z * 10.0f }; | ||
| accumulate_sample(field, _compass_instance); |
There was a problem hiding this comment.
| // Store in field vector and convert uT to milligauss | |
| Vector3f field { cr_ax_comp_x * 10.0f, cr_ax_comp_y * 10.0f, cr_ax_comp_z * 10.0f }; | |
| accumulate_sample(field, _compass_instance); | |
| // Store in field vector and convert uT to milligauss | |
| Vector3f field { cr_ax_comp_x, cr_ax_comp_y, cr_ax_comp_z }; | |
| field *= 10.0f; | |
| accumulate_sample(field, _compass_instance); |
There was a problem hiding this comment.
I did consider suggesting this change but decided against in this instance. For no particular reason - perhaps just for re-testing reasons.
peterbarker
force-pushed
the
pr/bmm350
branch
from
08bd34c
to
dc550bb
Compare
peterbarker
force-pushed
the
pr/bmm350
branch
from
dc550bb
to
cfdc537
Compare
|
@cuav-chen2 merged this one |
Replaceas #26987 as I could not force-push the branch