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

Add SPL06 barometer driver #19229

Merged
merged 11 commits into from
Mar 15, 2022
Merged

Add SPL06 barometer driver #19229

merged 11 commits into from
Mar 15, 2022

Conversation

Ncerzzk
Copy link
Contributor

@Ncerzzk Ncerzzk commented Feb 20, 2022

Please use PX4 Discuss or Slack to align on pull requests if necessary. You can then open draft pull requests to get early feedback.

Describe problem solved by this pull request
Added the driver of spl06 barometer. as the issue #13547

Describe your solution
it's a simple driver, just like how BMP280 implement.

Describe possible alternatives
A clear and concise description of alternative solutions or features you've considered.

Test data / coverage
I tested it on linux(SPI/I2C), while there may be problems on nuttx.
spl06

Additional context
if someone could test it on nuttx, it would be very nice. Because of this, I commented the spl06 dir in CMakeFiles, just for someone who really need this driver.

@dagar
Copy link
Member

dagar commented Feb 20, 2022

Is this the Goertek SPL06-007? https://datasheet.lcsc.com/szlcsc/1907081118_Goertek-SPL06-007_C233787.pdf

Could you move it under src/drivers/barometer/goertek/spl06? That's how I'm slowly reorganizing the drivers (eg drivers/imu).

@Ncerzzk Ncerzzk requested a review from dagar February 21, 2022 17:00
@Ncerzzk
Copy link
Contributor Author

Ncerzzk commented Feb 27, 2022

hi, @dagar do you have time to review this patch? Is there any other problems in the patch?
I'm afraid that you forgot this issue.
If you are busy doing some other work, please ignore the notice.

@dagar
Copy link
Member

dagar commented Feb 27, 2022

The change looks good. Do you intend to use this in a custom board or as an external peripheral on existing boards? If external might want to add a parameter like this to autostart.

# TE MS5525 differential pressure sensor external I2C
if param compare -s SENS_EN_MS5525 1
then
ms5525_airspeed start -X
fi

@Ncerzzk
Copy link
Contributor Author

Ncerzzk commented Feb 27, 2022

I added it behind the condition: SENS_EXT_I2C_PRB, is it OK?

@dagar
Copy link
Member

dagar commented Feb 27, 2022

I added it behind the condition: SENS_EXT_I2C_PRB, is it OK?

No, that doesn't scale well and there are potential conflicts that develop over time. It's better to do it explicitly now.

All you need to do is add a parameter, and then the little bit of code in rc.sensors.

/**
* TE MS5525 differential pressure sensor (external I2C)
*
* @reboot_required true
* @group Sensors
* @boolean
*/
PARAM_DEFINE_INT32(SENS_EN_MS5525, 0);

# TE MS5525 differential pressure sensor external I2C
if param compare -s SENS_EN_MS5525 1
then
ms5525_airspeed start -X
fi

@Ncerzzk
Copy link
Contributor Author

Ncerzzk commented Feb 27, 2022

I added it behind the condition: SENS_EXT_I2C_PRB, is it OK?

No, that doesn't scale well and there are potential conflicts that develop over time. It's better to do it explicitly now.

All you need to do is add a parameter, and then the little bit of code in rc.sensors.

/**
* TE MS5525 differential pressure sensor (external I2C)
*
* @reboot_required true
* @group Sensors
* @boolean
*/
PARAM_DEFINE_INT32(SENS_EN_MS5525, 0);

# TE MS5525 differential pressure sensor external I2C
if param compare -s SENS_EN_MS5525 1
then
ms5525_airspeed start -X
fi

OK, done.

@Ncerzzk
Copy link
Contributor Author

Ncerzzk commented Feb 28, 2022

removed -q option when starting spl06.

@Ncerzzk
Copy link
Contributor Author

Ncerzzk commented Mar 6, 2022

hi, @dagar , please review the changes.

@Ncerzzk Ncerzzk requested a review from dagar March 8, 2022 02:26
@Ncerzzk
Copy link
Contributor Author

Ncerzzk commented Mar 10, 2022

@dagar hi

@Ncerzzk
Copy link
Contributor Author

Ncerzzk commented Mar 14, 2022

can this patch be merged?

@dagar dagar merged commit cb23179 into PX4:master Mar 15, 2022
@dagar
Copy link
Member

dagar commented Mar 15, 2022

Thanks @Ncerzzk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants