fw/drivers: add BMP390 barometer driver and pressure/altitude API#1068
Open
mkoperator wants to merge 3 commits intocoredevices:mainfrom
Open
fw/drivers: add BMP390 barometer driver and pressure/altitude API#1068mkoperator wants to merge 3 commits intocoredevices:mainfrom
mkoperator wants to merge 3 commits intocoredevices:mainfrom
Conversation
Implement full BMP390 barometer driver with I2C communication, calibration data parsing, and configurable oversampling/filtering. Add pressure service applib layer exposing barometric pressure (Pa) and altitude (cm) to watchapps via the native SDK. Includes: - BMP390 driver: init, forced-mode reads, compensation math per datasheet - Pressure service: subscribe/unsubscribe, caching, ISA altitude calc - I2C burst read support for nRF5 platform - Console command for live pressure/altitude readout - Exported symbols for native SDK integration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pressure service functions are exported via the SDK jump table but ran in unprivileged app context, crashing when stored apps tried to access hardware drivers. Wrap all exported functions with DEFINE_SYSCALL and add a sys_pressure_read_and_compute syscall for the timer callback path. Also register the altimeter stored app in the system app registry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gmarull
requested changes
Apr 2, 2026
Member
gmarull
left a comment
gmarull
left a comment
There was a problem hiding this comment.
split changes into small chunks that can be reviewed easily, and follow commit guidelines.
src/fw/drivers/i2c/nrf5.c
Outdated
Comment on lines
+66
to
+70
| // Buffer for combining register address + write data into a single TX transfer. | ||
| // TXTX (two-part TX) on nRF52840 can fail to transmit the secondary buffer | ||
| // correctly due to TWIM peripheral errata. Using a single TX avoids this. | ||
| #define I2C_WRITE_BUF_MAX 32 | ||
| static uint8_t s_write_buf[I2C_WRITE_BUF_MAX]; |
Member
There was a problem hiding this comment.
red flag: static buffer shared for all instances, "peripheral errata", please, explain which one, and why doesn't nrfx cover it?
Author
I appreciate your warm welcome. Do you have anything more specific about how you would like these split? and as far as commit guidelines, only one I see is the signed off by on commits. Am I missing anything else? |
- Protect BMP390 reads/writes with a mutex for thread safety - Move nRF5 I2C write buffer from static global to per-bus state (errata coredevices#219 workaround now safe for multi-bus configs) - Add PressureFilterMode enum and pressure_set_filter_mode() driver API - Decouple IIR filter config from ODR presets — filter mode is now independently controllable - Add pressure_service_set_filter_mode() applib syscall + SDK export - Add BMP390 unit tests with fake I2C layer - Add standalone altimeter app skeleton Signed-off-by: Mikhail Kozorovitskiy <misha@mkoperator.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds support for the BMP390 barometric pressure sensor and exposes pressure/altitude data to watchapps through a new applib service.