Add HMC5883L 3-axis magnetometer sensor support #2998
Conversation
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
- Implement HMC5883L sensor driver with I2C communication - Add configurable gain, data rate, and sampling settings - Support magnetic field reading (X, Y, Z axes) in microTesla - Add heading calculation (0-360 degrees) and magnitude computation - Implement interactive calibration with min/max tracking - Add state management provider with real-time data collection - Create sensor UI screen with 5 real-time charts - Integrate HMC5883L into sensor selection navigation - Support periodic data collection with circular buffer - Include proper error handling and logging
Contributor
Reviewer's GuideAdds full HMC5883L magnetometer support by introducing a sensor driver with I2C configuration/reads, a Provider-based state manager for timed acquisition, calibration and buffering, and a new UI screen with charts and controls, then wiring it into the existing sensors navigation. Sequence diagram for HMC5883L initialization and periodic data acquisitionsequenceDiagram
actor User
participant SensorsScreen
participant HMC5883LScreen
participant HMC5883LProvider
participant Locator
participant ScienceLab
participant I2C
participant HMC5883L
User->>SensorsScreen: tap HMC5883L item
SensorsScreen->>HMC5883LScreen: navigate to HMC5883LScreen
activate HMC5883LScreen
HMC5883LScreen->>Locator: get ScienceLab
Locator-->>HMC5883LScreen: ScienceLab instance
HMC5883LScreen->>ScienceLab: isConnected
ScienceLab-->>HMC5883LScreen: true
HMC5883LScreen->>I2C: create with mPacketHandler
HMC5883LScreen->>HMC5883LProvider: create ChangeNotifier
activate HMC5883LProvider
HMC5883LScreen->>HMC5883LProvider: initializeSensors(onError, i2c, scienceLab)
HMC5883LProvider->>ScienceLab: isConnected
ScienceLab-->>HMC5883LProvider: true
HMC5883LProvider->>HMC5883L: create(i2c, scienceLab)
activate HMC5883L
HMC5883L->>ScienceLab: isConnected
ScienceLab-->>HMC5883L: true
HMC5883L->>I2C: readByte(address, idRegA)
I2C-->>HMC5883L: idA
HMC5883L->>I2C: readByte(address, idRegB)
I2C-->>HMC5883L: idB
HMC5883L->>I2C: readByte(address, idRegC)
I2C-->>HMC5883L: idC
HMC5883L->>I2C: write(address, [configA], configRegA)
HMC5883L->>I2C: write(address, [gain], configRegB)
HMC5883L->>I2C: write(address, [mode], modeReg)
HMC5883L-->>HMC5883LProvider: initialized instance
deactivate HMC5883L
HMC5883LProvider-->>HMC5883LScreen: notifyListeners
deactivate HMC5883LProvider
User->>HMC5883LScreen: press play in SensorControlsWidget
HMC5883LScreen->>HMC5883LProvider: toggleDataCollection
activate HMC5883LProvider
HMC5883LProvider->>HMC5883LProvider: _startDataCollection
HMC5883LProvider->>HMC5883LProvider: create periodic Timer
loop every timegapMs
HMC5883LProvider->>HMC5883L: getAllData
activate HMC5883L
HMC5883L->>HMC5883L: readRawData
HMC5883L->>I2C: readBulk(address, dataXMSB, 6)
I2C-->>HMC5883L: 6 data bytes
HMC5883L-->>HMC5883L: convert to signed x, y, z
HMC5883L->>HMC5883L: readMagneticField
HMC5883L->>HMC5883L: readHeading
HMC5883L->>HMC5883L: readMagnitude
HMC5883L-->>HMC5883LProvider: map with raw and computed values
deactivate HMC5883L
HMC5883LProvider->>HMC5883LProvider: update magneticX,Y,Z, heading, magnitude
HMC5883LProvider->>HMC5883LProvider: append ChartDataPoint to series
HMC5883LProvider-->>HMC5883LScreen: notifyListeners
HMC5883LScreen->>HMC5883LScreen: rebuild UI and charts
end
User->>HMC5883LScreen: press pause
HMC5883LScreen->>HMC5883LProvider: toggleDataCollection
HMC5883LProvider->>HMC5883LProvider: _stopDataCollection (cancel Timer)
deactivate HMC5883LProvider
Class diagram for new HMC5883L sensor supportclassDiagram
class HMC5883L {
<<sensor>>
+AppLocalizations appLocalizations
+static int address
+static int configRegA
+static int configRegB
+static int modeReg
+static int dataXMSB
+static int dataXLSB
+static int dataYMSB
+static int dataYLSB
+static int dataZMSB
+static int dataZLSB
+static int statusReg
+static int idRegA
+static int idRegB
+static int idRegC
+static int samplesAverage1
+static int samplesAverage2
+static int samplesAverage4
+static int samplesAverage8
+static int dataRate0_75Hz
+static int dataRate1_5Hz
+static int dataRate3Hz
+static int dataRate7_5Hz
+static int dataRate15Hz
+static int dataRate30Hz
+static int dataRate75Hz
+static int measurementNormal
+static int measurementPositiveBias
+static int measurementNegativeBias
+static int gain1370
+static int gain1090
+static int gain820
+static int gain660
+static int gain440
+static int gain390
+static int gain330
+static int gain230
+static int modeContinuous
+static int modeSingle
+static int modeIdle
+static Map~int,double~ gainScales
-I2C i2c
-int currentGain
-double scale
-double magneticX
-double magneticY
-double magneticZ
-double offsetX
-double offsetY
-double offsetZ
+HMC5883L._(I2C i2c)
+static Future~HMC5883L~ create(I2C i2c, ScienceLab scienceLab)
-Future~void~ _initialize(ScienceLab scienceLab)
+Future~void~ configure(int samplesAverage, int dataRate, int measurementMode, int gain, int mode)
+Future~Map~String,int~~ readRawData()
+Future~Map~String,double~~ readMagneticField()
+Future~double~ readHeading()
+Future~double~ readMagnitude()
+void setCalibrationOffsets(double minX, double maxX, double minY, double maxY, double minZ, double maxZ)
+Future~Map~String,dynamic~~ getAllData()
+Future~int~ readStatus()
+Future~bool~ isDataReady()
-int _toSignedInt16(int value)
+Future~void~ setGain(int gain)
+Future~void~ setMode(int mode)
}
class HMC5883LProvider {
<<ChangeNotifier>>
+AppLocalizations appLocalizations
-HMC5883L hmc5883l
-Timer dataTimer
-double magneticX
-double magneticY
-double magneticZ
-double heading
-double magnitude
-List~ChartDataPoint~ magneticXData
-List~ChartDataPoint~ magneticYData
-List~ChartDataPoint~ magneticZData
-List~ChartDataPoint~ headingData
-List~ChartDataPoint~ magnitudeData
-bool isRunning
-bool isLooping
-int timegapMs
-int numberOfReadings
-int collectedReadings
-bool isCalibrating
-double minX
-double maxX
-double minY
-double maxY
-double minZ
-double maxZ
-double currentTime
-static int maxDataPoints
+double get magneticX
+double get magneticY
+double get magneticZ
+double get heading
+double get magnitude
+List~ChartDataPoint~ get magneticXData
+List~ChartDataPoint~ get magneticYData
+List~ChartDataPoint~ get magneticZData
+List~ChartDataPoint~ get headingData
+List~ChartDataPoint~ get magnitudeData
+bool get isRunning
+bool get isLooping
+bool get isCalibrating
+int get timegapMs
+int get numberOfReadings
+int get collectedReadings
+HMC5883LProvider()
+Future~void~ initializeSensors(Function onError, I2C i2c, ScienceLab scienceLab)
+void toggleDataCollection()
-void _startDataCollection()
-void _stopDataCollection()
-Future~void~ _fetchSensorData()
-void _updateCalibrationData(double x, double y, double z)
+void startCalibration()
+void stopCalibration()
+String getCalibrationStatus()
+void setTimegap(int ms)
+void setNumberOfReadings(int count)
+void toggleLooping()
+void clearData()
+Future~void~ setGain(int gain)
+void dispose()
}
class HMC5883LScreen {
<<StatefulWidget>>
+String sensorImage
-I2C i2c
-ScienceLab scienceLab
-HMC5883LProvider provider
+HMC5883LScreen()
+State createState()
}
class _HMC5883LScreenState {
-AppLocalizations appLocalizations
-String sensorImage
-I2C i2c
-ScienceLab scienceLab
-HMC5883LProvider provider
+void initState()
-void _initializeScienceLab()
-void _showSensorErrorSnackbar(String message)
-void _showSuccessSnackbar(String message)
+Widget build(BuildContext context)
-Widget _buildRawDataSection(HMC5883LProvider provider)
-Widget _buildCalibrationSection(HMC5883LProvider provider)
-void _showGainDialog(HMC5883LProvider provider)
-Widget _buildDataCard(String label, String value)
+void dispose()
}
class ScienceLab {
+bool isConnected()
+dynamic mPacketHandler
}
class I2C {
+I2C(dynamic packetHandler)
+Future~int~ readByte(int address, int register)
+Future~List~int~~ readBulk(int address, int startRegister, int length)
+Future~void~ write(int address, List~int~ data, int register)
}
class ChartDataPoint {
+double x
+double y
+ChartDataPoint(double x, double y)
}
class CommonScaffold {
+Widget body
+String title
}
class SensorChartWidget {
+String title
+String yAxisLabel
+List~ChartDataPoint~ data
+Color lineColor
+String unit
+int maxDataPoints
+bool showDots
}
class SensorControlsWidget {
+bool isPlaying
+bool isLooping
+int timegapMs
+int numberOfReadings
+Function onPlayPause
+Function onLoop
+Function onTimegapChanged
+Function onNumberOfReadingsChanged
+Function onClearData
}
HMC5883LProvider --> HMC5883L : uses
HMC5883LProvider --> ChartDataPoint : creates
HMC5883LScreen ..> HMC5883LProvider : ChangeNotifierProvider
HMC5883LScreen ..> SensorChartWidget : composes
HMC5883LScreen ..> SensorControlsWidget : composes
HMC5883LScreen ..> CommonScaffold : uses
_HMC5883LScreenState --> HMC5883LProvider : holds
_HMC5883LScreenState --> ScienceLab : uses
_HMC5883LScreenState --> I2C : uses
HMC5883L --> I2C : uses
HMC5883L --> ScienceLab : uses
SensorsScreen ..> HMC5883LScreen : navigates
class SensorsScreen {
+Widget build(BuildContext context)
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The
HMC5883LScreeninitializes_i2c/_scienceLabasynchronously in_initializeScienceLab, but theChangeNotifierProvider.createimmediately callsinitializeSensorswith these fields; this can result in a null I2C/ScienceLab race — consider awaiting initialization before creating the provider or passing the dependencies directly once they’re ready. HMC5883L.getAllDataredundantly callsreadRawData/readMagneticField/readHeading/readMagnitude, each of which can trigger additional I2C reads; refactor to read the sensor once and derive all computed values from that single measurement to reduce bus traffic and latency.- In
HMC5883LScreen, storing the provider in a field and explicitly calling_provider.dispose()is unnecessary and risks double-dispose — you can rely onChangeNotifierProviderto manage the lifecycle and access the provider viacontext.read/context.watchinstead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `HMC5883LScreen` initializes `_i2c`/`_scienceLab` asynchronously in `_initializeScienceLab`, but the `ChangeNotifierProvider.create` immediately calls `initializeSensors` with these fields; this can result in a null I2C/ScienceLab race — consider awaiting initialization before creating the provider or passing the dependencies directly once they’re ready.
- `HMC5883L.getAllData` redundantly calls `readRawData`/`readMagneticField`/`readHeading`/`readMagnitude`, each of which can trigger additional I2C reads; refactor to read the sensor once and derive all computed values from that single measurement to reduce bus traffic and latency.
- In `HMC5883LScreen`, storing the provider in a field and explicitly calling `_provider.dispose()` is unnecessary and risks double-dispose — you can rely on `ChangeNotifierProvider` to manage the lifecycle and access the provider via `context.read`/`context.watch` instead.
## Individual Comments
### Comment 1
<location> `lib/view/hmc5883l_screen.dart:18-27` </location>
<code_context>
+ }
+ }
+
+ @override
+ void dispose() {
+ _dataTimer?.cancel();
</code_context>
<issue_to_address>
**issue (bug_risk):** Manually disposing the provider risks double-dispose with ChangeNotifierProvider.
Let `ChangeNotifierProvider` own the lifecycle and remove any manual `_provider.dispose()` calls from this State’s `dispose` method to avoid double-dispose issues and potential runtime exceptions.
</issue_to_address>
### Comment 2
<location> `lib/communication/sensors/hmc5883l.dart:236-241` </location>
<code_context>
+ "X=$offsetX, Y=$offsetY, Z=$offsetZ");
+ }
+
+ Future<Map<String, dynamic>> getAllData() async {
+ try {
+ final rawData = await readRawData();
+ await readMagneticField();
+ final heading = await readHeading();
+ final magnitude = await readMagnitude();
+
+ return {
</code_context>
<issue_to_address>
**suggestion (performance):** getAllData performs multiple redundant I2C reads, which is inefficient.
`getAllData` calls `readRawData()`, then `readMagneticField()`, and then `readHeading()` and `readMagnitude()`, both of which call `readMagneticField()` (and thus `readRawData()`) again. This causes multiple I2C transactions per fetch. Consider restructuring so raw data is read once and used to compute magnetic field, heading, and magnitude, e.g. via a helper that accepts the raw data or `magneticX/Y/Z`. This will reduce bus traffic and latency for periodic sampling.
Suggested implementation:
```
final magneticField = await readMagneticField(rawData: rawData);
final heading = await readHeading(magneticField: magneticField);
final magnitude = await readMagnitude(magneticField: magneticField);
```
To fully implement this optimization, you should also adjust the sensor-reading methods so they can reuse already-fetched data instead of always hitting the I2C bus:
1. **Update `readMagneticField` to accept raw data**:
- Change the signature roughly from:
```dart
Future<Map<String, double>> readMagneticField() async {
```
to:
```dart
Future<Map<String, double>> readMagneticField({Map<String, int>? rawData}) async {
```
- Inside, replace the internal `final rawData = await readRawData();` with:
```dart
final data = rawData ?? await readRawData();
```
- Then use `data['x']`, `data['y']`, `data['z']` instead of the previous local `rawData` variable.
2. **Update `readHeading` to accept a precomputed magnetic field**:
- Change the signature from something like:
```dart
Future<double> readHeading() async {
```
to:
```dart
Future<double> readHeading({Map<String, double>? magneticField}) async {
```
- Replace the internal `final magneticField = await readMagneticField();` with:
```dart
final field = magneticField ?? await readMagneticField();
```
- Use `field['x']`, `field['y']` (and `field['z']` if needed) where you previously used `magneticField`.
3. **Update `readMagnitude` similarly**:
- Change the signature from:
```dart
Future<double> readMagnitude() async {
```
to:
```dart
Future<double> readMagnitude({Map<String, double>? magneticField}) async {
```
- Replace `final magneticField = await readMagneticField();` with:
```dart
final field = magneticField ?? await readMagneticField();
```
- Use `field` instead of the old local variable.
4. **Check call sites**:
- Existing call sites that do not pass parameters (e.g. `await readMagneticField()`, `await readHeading()`, `await readMagnitude()`) will still work because the new parameters are optional and default to doing the same I2C reads as before.
- The only place that will now avoid redundant I2C reads is `getAllData`, which passes `rawData` and `magneticField` explicitly as shown in the edit block above.
These changes ensure that `getAllData` performs a single raw I2C read and reuses that data through the stack, reducing bus traffic and improving sampling latency without breaking existing usages.
</issue_to_address>
### Comment 3
<location> `lib/view/hmc5883l_screen.dart:464-476` </location>
<code_context>
+ context: context,
+ builder: (context) => AlertDialog(
+ title: const Text('Select Gain'),
+ content: Column(
+ mainAxisSize: MainAxisSize.min,
+ children: gains.entries.map((entry) {
+ return ListTile(
+ title: Text(entry.key),
+ onTap: () {
+ provider.setGain(entry.value);
+ Navigator.pop(context);
+ _showSuccessSnackbar('Gain set to ${entry.key}');
+ },
+ );
+ }).toList(),
+ ),
+ actions: [
</code_context>
<issue_to_address>
**suggestion:** Gain selection dialog content can overflow vertically when the list is long.
Because the dialog’s `content` is a `Column` with all `gains.entries` as `ListTile`s, it can exceed the available height on small screens or with larger text scales and overflow. Consider constraining and scrolling the list (e.g. wrap it in a `SizedBox` with a `ListView` or `SingleChildScrollView`) so the dialog remains usable on smaller viewports and with accessibility settings.
```suggestion
content: ConstrainedBox(
constraints: const BoxConstraints(
// Limit max height so the dialog remains usable on small screens
maxHeight: 300,
),
child: ListView(
shrinkWrap: true,
children: gains.entries.map((entry) {
return ListTile(
title: Text(entry.key),
onTap: () {
provider.setGain(entry.value);
Navigator.pop(context);
_showSuccessSnackbar('Gain set to ${entry.key}');
},
);
}).toList(),
),
),
```
</issue_to_address>
### Comment 4
<location> `lib/providers/hmc5883l_provider.dart:89-90` </location>
<code_context>
+ logger.d("HMC5883L sensor initialized successfully");
+ notifyListeners();
+ } catch (e) {
+ logger.e('Error initializing HMC5883L: $e');
+ onError('Error initializing HMC5883L: $e');
+ }
+ }
</code_context>
<issue_to_address>
**suggestion:** Error message mixes localized and raw text, which can be inconsistent for users.
Here you pass a hard-coded English string with the exception to `onError`, whereas elsewhere you use `appLocalizations.pslabNotConnected`. For consistency and proper localization, use a localized base message here as well and, if needed, append `e.toString()` in a structured way or map common failures to localized user-friendly messages.
Suggested implementation:
```
} catch (e, stackTrace) {
// Log a structured error with exception and stack trace
logger.e('Error initializing HMC5883L', e, stackTrace);
// Use a localized base message and append the raw error in a structured way
final baseMessage = appLocalizations.hmc5883lInitError;
onError('$baseMessage\n(${e.toString()})');
}
```
1. Add a new localization key such as `hmc5883lInitError` to your ARB/JSON localization files, e.g. `"hmc5883lInitError": "Error initializing HMC5883L sensor"`, and regenerate `appLocalizations`.
2. If your logger's `e` method does not accept `(message, error, stackTrace)`, adjust the call to match its existing signature while still logging both the message and `e`.
3. If you prefer not to expose raw exception text to end users, replace `\n(${e.toString()})` with a more user-friendly localized hint or omit it entirely.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- Fix redundant I2C reads in getAllData by accepting optional parameters in readMagneticField, readHeading, and readMagnitude methods - Optimize sensor data collection to perform single I2C read and reuse data through computation stack - Add scrollable ConstrainedBox to gain selection dialog to prevent overflow on small screens - Improve error message localization using magnetometerError from app localizations - Fix provider lifecycle management by removing manual disposal and letting ChangeNotifierProvider handle it - Remove unnecessary _provider field storage in HMC5883LScreen Addresses review comments: - Performance: Reduces bus traffic and latency for periodic sampling - UI: Dialog remains usable on all screen sizes with accessibility settings - UX: Consistent localized error messages - Architecture: Proper provider lifecycle management
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #2991
This pull request adds support for the HMC5883L 3-axis magnetometer sensor to the PSLab application. The implementation includes a complete sensor driver, state management, and an interactive UI for real-time data visualization and calibration.
Changes
New Files
lib/communication/sensors/hmc5883l.dart- Low-level sensor driver handling I2C communication with the HMC5883L magnetometerlib/providers/hmc5883l_provider.dart- State management using Provider patternlib/view/hmc5883l_screen.dart- Complete sensor UI with visualizationModified Files
lib/view/sensors_screen.dart- Added navigation route for HMC5883L sensorHow It Works
Technical Details
atan2(magneticY, magneticX)converted to 0-360° rangesqrt(X² + Y² + Z²)for the total field strengthScreenshots / Recordings
Checklist
flutter analyze, all null safety checks passedRelated Issue
Closes #2991
Summary by Sourcery
Add support for the HMC5883L 3-axis magnetometer, including a sensor driver, provider-based state management, and a dedicated UI screen with real-time visualization and calibration controls.
New Features: