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

Pitot DLVR-L10D sensor #9216

Merged
merged 18 commits into from
Aug 30, 2023
Merged

Pitot DLVR-L10D sensor #9216

merged 18 commits into from
Aug 30, 2023

Conversation

jmajew
Copy link
Contributor

@jmajew jmajew commented Aug 5, 2023

This PR (related issue #9208):

  • adds a driver for DLVR-L10D sensor which is close to the MS4525 however the pressure treatment is different.

  • modifies reading algo of pitot sensors such that measured period of execution follows closely the demanded one.

  • fixes the problem when LPF affected the zero-calibration of noisy sensors (e.g., MS4525 especially with PCB ver.1.1). such that for high cutoff frequencies it could keep zero-calibration procedure running for ever

  • most of modifications follow closely those that are used in ArduPlane

  • I have tested the code using FC Matek H743-WLITE and I2C sensors MS4525 (PCB ver.1.1, PCB ver.1.3) and Matek AS-DLVR-L10D I2C. Analog sensors (I do not have one) should be checked before merging.

@@ -74,6 +74,13 @@ static bool ms4525_read(pitotDev_t * pitot)
dP_raw2 = 0x3FFF & ((rxbuf2[0] << 8) + rxbuf2[1]);
dT_raw2 = (0xFFE0 & ((rxbuf2[2] << 8) + rxbuf2[3])) >> 5;

// reject any values that are the absolute minimum or maximums these
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these changes needed for the new sensor, or an unrelated change made during development?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is additional safeguard taken directly from ArduPlane MS4525 driver (AP_Airspeed_MS4525.cpp line 180) to ensure that read values are correct. I guess it rarely is hit. And yes, it is unrelated.

@@ -494,7 +494,7 @@ cfTask_t cfTasks[TASK_COUNT] = {
[TASK_BARO] = {
.taskName = "BARO",
.taskFunc = taskUpdateBaro,
.desiredPeriod = TASK_PERIOD_HZ(20),
.desiredPeriod = TASK_PERIOD_MS(50), // SPL06 uses oversampilng - result 16ms ?
Copy link
Collaborator

Choose a reason for hiding this comment

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

20Hz ends up being 50ms. Is this change needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really - it can be dropped. I just did this modification because it was easier for me to understand difference between what baro wants and what it gets (it interfered with pitot measurements since the true period for SPL06 is 16ms).

Copy link
Member

Choose a reason for hiding this comment

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

@jmajew please remove it if it's not required for those changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

} else {
pitot.pressure = pitotPressureTmp;
Copy link
Collaborator

@mmosca mmosca Aug 5, 2023

Choose a reason for hiding this comment

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

why are we using the pressure at this point, when the calibration is not complete and we are also setting airspeed to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before the if (pitotIsCalibrationComplete()) { (line 253) the pressure dP is kept in variable pitotPressureTmp.
During normal operation pitot.pressure is initialized by the LPF function (line 266) which takes as an argument the pitotPressureTmp.
During zero calibration which relies on pitot.pressure it is not initialized since LPF has been moved to normal operation part of if. Hence the need for this line.

@mmosca
Copy link
Collaborator

mmosca commented Aug 5, 2023

Cool! Nice contribution.

I see you skipped the sensor during auto detection.

So, it identifies itself as ms4525 over i2c, but requires different reading for the data.

Is there any register that can differentiate the two of them, so auto detection can be used?

@jmajew
Copy link
Contributor Author

jmajew commented Aug 5, 2023

Is there any register that can differentiate the two of them, so auto detection can be used?

Not that I can find... In fact you can run both MS4525 and DLVR-L10D using both drivers usually without problem. Apart from the fact that results would be wrong.

@DzikuVx DzikuVx added this to the 7.0 milestone Aug 7, 2023
@jmajew
Copy link
Contributor Author

jmajew commented Aug 12, 2023

I have applied this PR to 6.1.1 version of the firmware and used it to test DLVR-L10D sensor during real flight 2 days ago. My personal feeling were very positive: no noise (standard setting for filtering 350mHz), stable measured airspeed and the value of the airspeed looks reasonable comparing to wind and GPS speeds. Subjectively I would say it behaves much better than what I remember when using MS4525 on the same plane.

I would like to test also the PR to see how MS4525 sensor work after modifications but unfortunately next chance to fly for me would be around the end of August :(

@DzikuVx DzikuVx added the Release Notes Add this when a PR needs to be mentioned in the release notes label Aug 30, 2023
@DzikuVx DzikuVx merged commit cd364cf into iNavFlight:master Aug 30, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release Notes Add this when a PR needs to be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants