feat: Add support for CCS811 sensor#3032
feat: Add support for CCS811 sensor#3032soumyajitnandi0 wants to merge 10 commits intofossasia:flutterfrom
Conversation
Reviewer's GuideImplements CCS811 air quality sensor support by adding a sensor driver, provider layer, configuration model, and a new UI screen with live charts and CSV recording, and wiring it into the existing sensors navigation. Sequence diagram for CCS811 sensor initialization and periodic data collectionsequenceDiagram
actor User
participant SensorsScreen
participant Navigator
participant CCS811Screen
participant _CCS811ScreenState
participant MultiProvider
participant CCS811ConfigProvider
participant CCS811Provider
participant ScienceLab
participant I2C
participant CCS811
User->>SensorsScreen: tap CCS811 sensor item
SensorsScreen->>Navigator: push CCS811Screen
Navigator->>CCS811Screen: create
CCS811Screen->>_CCS811ScreenState: createState
_CCS811ScreenState->>_CCS811ScreenState: initState
_CCS811ScreenState->>_CCS811ScreenState: _initializeScienceLab()
_CCS811ScreenState->>ScienceLab: getIt.get
ScienceLab-->>_CCS811ScreenState: ScienceLab instance
_CCS811ScreenState->>I2C: create with mPacketHandler
I2C-->>_CCS811ScreenState: I2C instance
_CCS811ScreenState->>MultiProvider: build
MultiProvider->>CCS811ConfigProvider: create
MultiProvider->>CCS811Provider: create(configProvider)
CCS811Provider->>CCS811ConfigProvider: addListener(_onConfigChanged)
_CCS811ScreenState->>CCS811Provider: initializeSensors(onError, i2c, scienceLab)
CCS811Provider->>ScienceLab: isConnected()
ScienceLab-->>CCS811Provider: true
CCS811Provider->>CCS811: create(i2c, scienceLab)
activate CCS811
CCS811->>ScienceLab: isConnected()
ScienceLab-->>CCS811: true
CCS811->>CCS811: _initialize(scienceLab)
CCS811->>CCS811: _readRegisterByte(hwId)
CCS811-->>CCS811Provider: CCS811 instance
deactivate CCS811
CCS811Provider->>CCS811Provider: _startDataCollection()
CCS811Provider-->>_CCS811ScreenState: sensorAvailable = true
loop every updatePeriod ms
CCS811Provider->>CCS811: getRawData()
CCS811->>CCS811: _readRegisterByte(status)
CCS811->>CCS811: readBulk(algResultData)
CCS811-->>CCS811Provider: {eCO2, TVOC}
CCS811Provider->>CCS811Provider: _updateCharts()
CCS811Provider->>CCS811Provider: _recordDataIfEnabled()
CCS811Provider-->>CCS811Screen: notifyListeners
end
Updated class diagram for CCS811 sensor supportclassDiagram
class CCS811Screen {
+CCS811Screen()
}
class _CCS811ScreenState {
+AppLocalizations appLocalizations
-CsvService _csvService
-CCS811Provider _provider
-I2C _i2c
-ScienceLab _scienceLab
-CCS811ConfigProvider _configProvider
+initState()
-_initializeScienceLab()
-_showSensorErrorSnackbar(message String)
-_navigateToLoggedData() Future~void~
-_toggleRecording() Future~void~
-_showSaveFileDialog(data List~List~dynamic~~) Future~void~
+build(context BuildContext) Widget
-_buildValueCard(title String, value String, unit String) Widget
-_buildChart(spots List~FlSpot~, title String, color Color) Widget
}
class CCS811Provider {
+AppLocalizations appLocalizations
-CCS811ConfigProvider _configProvider
-int _currentECO2
-int _currentTVOC
-List~double~ _eCO2Data
-List~double~ _tvocData
-List~double~ _timeData
-List~FlSpot~ eCO2ChartData
-List~FlSpot~ tvocChartData
-Timer _dataTimer
-StreamSubscription _locationStream
-Position currentPosition
-double _startTime
-double _currentTime
-int _chartMaxLength
-int _eCO2Min
-int _eCO2Max
-int _tvocMin
-int _tvocMax
-bool _sensorAvailable
-bool _isRecording
-List~List~dynamic~~ _recordedData
-CCS811 _ccs811
-I2C _i2c
-ScienceLab _scienceLab
-Function(String) onSensorError
+CCS811Provider(configProvider CCS811ConfigProvider)
+initializeSensors(onError Function(String), i2c I2C, scienceLab ScienceLab) Future~void~
+sensorAvailable bool
+isRecording bool
+startRecording() Future~void~
+stopRecording() void
+getRecordedData() List~List~dynamic~~
+disposeSensors() void
+currentECO2 int
+currentTVOC int
+getECO2ChartData() List~FlSpot~
+getTVOCChartData() List~FlSpot~
+getMinTime() double
+getMaxTime() double
+getTimeInterval() double
+dispose() void
-_onConfigChanged() void
-_reinitializeSensors() void
-_startDataCollection() void
-_readData() Future~void~
-_updateCharts() void
-_recordDataIfEnabled() void
-_startGeoLocationUpdates() Future~void~
}
class CCS811ConfigProvider {
-CCS811Config _config
+CCS811ConfigProvider()
+config CCS811Config
+updateConfig(newConfig CCS811Config) void
+updateUpdatePeriod(updatePeriod int) void
+updateIncludeLocationData(includeLocationData bool) void
+resetToDefaults() void
-_loadConfigFromPrefs() Future~void~
-_saveConfigToPrefs() Future~void~
}
class CCS811Config {
+int updatePeriod
+bool includeLocationData
+CCS811Config(updatePeriod int, includeLocationData bool)
+copyWith(updatePeriod int, includeLocationData bool) CCS811Config
+fromJson(json Map~String,dynamic~) CCS811Config
+toJson() Map~String,dynamic~
}
class CCS811 {
+static String tag
+static int address
+static int status
+static int measMode
+static int algResultData
+static int rawData
+static int envData
+static int ntc
+static int thresholds
+static int baseline
+static int hwId
+static int hwVersion
+static int fwBootVersion
+static int fwAppVersion
+static int errorId
+static int appStart
+static int swReset
+static int modeIdle
+static int mode1s
+static int mode10s
+static int mode60s
+static int mode250ms
+static String name
+I2C i2c
-int _eCO2
-int _tVOC
+CCS811._(i2c I2C)
+create(i2c I2C, scienceLab ScienceLab) Future~CCS811~
+setMode(mode int) Future~void~
+getRawData() Future~Map~String,int~~
-_initialize(scienceLab ScienceLab) Future~void~
-_readRegisterByte(register int) Future~int~
}
class ChangeNotifier
class Equatable
class I2C
class ScienceLab
class FlSpot
class Position
CCS811Screen --> _CCS811ScreenState : creates
_CCS811ScreenState ..> CCS811Provider : uses
_CCS811ScreenState ..> CCS811ConfigProvider : uses
_CCS811ScreenState ..> CsvService : uses
_CCS811ScreenState ..> LoggedDataScreen : navigates
CCS811Provider --|> ChangeNotifier
CCS811ConfigProvider --|> ChangeNotifier
CCS811Config --|> Equatable
CCS811Provider o--> CCS811ConfigProvider : holds_reference
CCS811Provider o--> CCS811 : uses_sensor
CCS811Provider o--> I2C
CCS811Provider o--> ScienceLab
CCS811Provider ..> FlSpot
CCS811Provider ..> Position
CCS811ConfigProvider o--> CCS811Config
CCS811 o--> I2C
CCS811 ..> ScienceLab
File-Level Changes
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}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- There’s a potential race between
_initializeScienceLab()(async) and theCCS811Provider.initializeSensorscall in the post-frame callback – if_i2c/_scienceLabaren’t ready yet you’ll hit the 'ScienceLab not connected' path; consider awaiting initialization before wiring up the provider or deferringinitializeSensorsuntil the lab is confirmed connected. - Several user-facing strings are hard-coded in the new code (e.g. snack bar messages, 'ScienceLab not connected', 'Failed to initialize CCS811...', 'CCS811 Air Quality'); these should be sourced from
AppLocalizationsor existing constants to follow the project’s no hard-coding/localization pattern. - There are a few unused or effectively dead elements (e.g.
AppLocalizations appLocalizationsinCCS811and_navigateToLoggedDatainCCS811Screen) that can be removed or wired up to avoid carrying unused code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a potential race between `_initializeScienceLab()` (async) and the `CCS811Provider.initializeSensors` call in the post-frame callback – if `_i2c`/`_scienceLab` aren’t ready yet you’ll hit the 'ScienceLab not connected' path; consider awaiting initialization before wiring up the provider or deferring `initializeSensors` until the lab is confirmed connected.
- Several user-facing strings are hard-coded in the new code (e.g. snack bar messages, 'ScienceLab not connected', 'Failed to initialize CCS811...', 'CCS811 Air Quality'); these should be sourced from `AppLocalizations` or existing constants to follow the project’s no hard-coding/localization pattern.
- There are a few unused or effectively dead elements (e.g. `AppLocalizations appLocalizations` in `CCS811` and `_navigateToLoggedData` in `CCS811Screen`) that can be removed or wired up to avoid carrying unused code.
## Individual Comments
### Comment 1
<location> `lib/providers/ccs811_provider.dart:96-97` </location>
<code_context>
+ void _startDataCollection() {
+ int interval = _configProvider.config.updatePeriod;
+ _dataTimer?.cancel();
+ _dataTimer = Timer.periodic(Duration(milliseconds: interval), (timer) async {
+ await _readData();
+ });
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid using an async callback directly in Timer.periodic to prevent overlapping sensor reads.
Because `Timer.periodic` doesn’t wait for the `async` callback, slow I2C operations or a short `updatePeriod` can cause `_readData` calls to overlap, leading to concurrent access to `_ccs811` and the internal data arrays. Guard `_readData` with a `_isReading` flag, or refactor so each read schedules the next timer only after it completes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
soumyajitnandi0
force-pushed
the
flutter
branch
from
7139917 to
9a9e40c
Compare
soumyajitnandi0
changed the title
soumyajitnandi0
force-pushed
the
flutter
branch
4 times, most recently
from
28ca6df to
c3b803b
Compare
Build StatusBuild successful. APKs to test: https://github.com/fossasia/pslab-app/actions/runs/22302288793/artifacts/5615170216. ScreenshotsAndroid Screenshots
iPhone Screenshots
iPad Screenshots
|
||||||||||||||||||||||||
-1_instruments_screen.png?raw=true)
-2_nav_drawer.png?raw=true)
-3_accelerometer.png?raw=true)
-4_power_source.png?raw=true)
-5_multimeter.png?raw=true)
-6_wave_generator.png?raw=true)
-7_oscilloscope.png?raw=true)
soumyajitnandi0
force-pushed
the
flutter
branch
from
c3b803b to
a9f450d
Compare
soumyajitnandi0
force-pushed
the
flutter
branch
from
a9f450d to
fa1cc40
Compare
soumyajitnandi0
force-pushed
the
flutter
branch
2 times, most recently
from
dc99560 to
fa1cc40
Compare
|
@marcnause |
marcnause
changed the title
There was a problem hiding this comment.
Pull request overview
Adds CCS811 air quality sensor support to the PSLab Flutter app, including the I2C driver, provider/state handling, and a dedicated UI screen with charting and CSV recording (optionally with geolocation).
Changes:
- Added CCS811 I2C driver and a config model/provider for sampling & recording options.
- Implemented CCS811 data provider with periodic reads, chart series generation, and CSV logging (with optional location).
- Added a CCS811 UI screen and wired it into the sensor selection flow.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pubspec.yaml | Adds equatable (and includes existing geolocator/wakelock deps) to support new config model. |
| pubspec.lock | Updates resolved package set and SDK constraints after dependency changes. |
| macos/Flutter/GeneratedPluginRegistrant.swift | Registers additional plugins (geolocator, wakelock) for macOS. |
| lib/view/sensors_screen.dart | Adds CCS811 to the sensor list and routes to the new screen. |
| lib/view/ccs811_screen.dart | New CCS811 UI showing live values, charts, and CSV recording controls. |
| lib/providers/ccs811_provider.dart | New provider for CCS811 polling, chart data, recording, and geolocation stream hookup. |
| lib/providers/ccs811_config_provider.dart | Persists CCS811 settings via SharedPreferences. |
| lib/models/ccs811_config.dart | New equatable config model (update period + include location). |
| lib/communication/sensors/ccs811.dart | New CCS811 I2C driver (init/app start, mode, data reads). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| targetScreen = const VL53L0XScreen(); | ||
| break; | ||
| case 'CCS811': | ||
| targetScreen = const CCS811Screen(); |
There was a problem hiding this comment.
Missing break after the CCS811 case causes fall-through into the SHT21 case, so tapping CCS811 will navigate to the SHT21 screen instead. Add a break (or return) after setting targetScreen for CCS811.
| targetScreen = const CCS811Screen(); | |
| targetScreen = const CCS811Screen(); | |
| break; |
| environment: | ||
| sdk: ^3.5.4 | ||
| flutter: '3.38.7' | ||
| flutter: "3.38.7" |
There was a problem hiding this comment.
pubspec.lock resolves with dart: ">=3.9.0 <4.0.0", but pubspec.yaml still allows sdk: ^3.5.4. This mismatch can cause dependency resolution/build failures for anyone on Dart 3.5.x. Update the environment.sdk constraint to the actual minimum Dart required by the resolved dependencies (likely aligned with Flutter 3.38.7’s bundled Dart).
lib/providers/ccs811_provider.dart
Outdated
|
|
||
| bool get sensorAvailable => _sensorAvailable; | ||
| bool get isRecording => _isRecording; | ||
|
|
||
| CCS811Provider(this._configProvider) { | ||
| _configProvider.addListener(_onConfigChanged); | ||
| } | ||
|
|
||
| void _onConfigChanged() { | ||
| if (_sensorAvailable) { | ||
| _reinitializeSensors(); |
There was a problem hiding this comment.
_onConfigChanged reinitializes the CCS811 hardware for any config change, but the only config fields are updatePeriod and includeLocationData—changing location logging shouldn’t require tearing down and re-creating the sensor. Track the previous config and only restart the timer / sensor when the sampling interval changes.
| bool get sensorAvailable => _sensorAvailable; | |
| bool get isRecording => _isRecording; | |
| CCS811Provider(this._configProvider) { | |
| _configProvider.addListener(_onConfigChanged); | |
| } | |
| void _onConfigChanged() { | |
| if (_sensorAvailable) { | |
| _reinitializeSensors(); | |
| Object? _lastUpdatePeriod; | |
| bool get sensorAvailable => _sensorAvailable; | |
| bool get isRecording => _isRecording; | |
| CCS811Provider(this._configProvider) { | |
| _lastUpdatePeriod = _configProvider.updatePeriod; | |
| _configProvider.addListener(_onConfigChanged); | |
| } | |
| void _onConfigChanged() { | |
| if (_sensorAvailable) { | |
| final currentPeriod = _configProvider.updatePeriod; | |
| if (currentPeriod != _lastUpdatePeriod) { | |
| _lastUpdatePeriod = currentPeriod; | |
| _reinitializeSensors(); | |
| } |
| Future<void> _startGeoLocationUpdates() async { | ||
| bool serviceEnabled = await Geolocator.isLocationServiceEnabled(); | ||
| if (!serviceEnabled) return; | ||
|
|
||
| _locationStream = Geolocator.getPositionStream( | ||
| locationSettings: const LocationSettings(accuracy: LocationAccuracy.high), | ||
| ).listen((Position position) { | ||
| currentPosition = position; | ||
| }); | ||
| } |
There was a problem hiding this comment.
_startGeoLocationUpdates starts a position stream without checking/requesting permissions. In this codebase, other providers (e.g. BarometerStateProvider) call Geolocator.checkPermission() / requestPermission() and handle deniedForever; without that, this can throw and break recording when location is enabled. Add the same permission flow here before subscribing.
| return CommonScaffold( | ||
| title: "CCS811 Air Quality", | ||
| onRecordPressed: _toggleRecording, |
There was a problem hiding this comment.
This screen introduces several user-facing strings as hardcoded text (e.g., the scaffold title). Since the app is localized, these should be moved into the ARB files and accessed via AppLocalizations (and regenerated app_localizations*.dart) so they translate correctly.
lib/providers/ccs811_provider.dart
Outdated
| _scienceLab = scienceLab; | ||
|
|
||
| if (_i2c == null || _scienceLab == null || !_scienceLab!.isConnected()) { | ||
| onSensorError?.call('ScienceLab not connected'); |
There was a problem hiding this comment.
initializeSensors reports connection failures using a hardcoded string ('ScienceLab not connected') even though appLocalizations.pslabNotConnected exists and is used elsewhere. Consider using the localized string here for consistent UX across screens.
| onSensorError?.call('ScienceLab not connected'); | |
| onSensorError?.call(appLocalizations.pslabNotConnected); |
|
Thank you for adding support for the CCS811 air quality sensor and for your work on this feature. The implementation of the sensor driver, provider, UI screen with live charts, and CSV recording is a valuable addition. Before this can be merged, a few issues need to be addressed: 1. Resolve merge conflicts and update to the current codebase 2. Address outstanding review feedback
Please resolve these points and push an update. Once the branch is up to date with Thank you again for your contribution and for improving PSLab’s sensor support. |
mariobehling
left a comment
mariobehling
left a comment
There was a problem hiding this comment.
Please address AI comments.
Also a process note.
We have automatic Copilot PR reviews enabled on this repository. These reviews are only triggered if the contributor has GitHub Copilot enabled and an active license on their own account.
Please enable Copilot in your GitHub settings if you have access. In many regions, free licenses are available through educational institutions or developer programs. Enabling Copilot helps us speed up the auto review process and reduces manual review overhead for the core team.
Fixes #2990
Changes
CCS811ConfigProviderfor settings.Checklist:
constants.dartor localization files instead of hard-coded values (where applicable).dart format.flutter analyzeand tests run influtter test.