feat: ported sensors screen and BMP180 screen #2789
Conversation
Reviewer's GuidePorts the sensors and BMP180 functionality into the Flutter app by adding new localization resources, theming, navigation routes, UI screens, reusable chart and control widgets, a BMP180 data provider, and a chart data model. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Build successful. APKs to test: https://github.com/fossasia/pslab-app/actions/runs/16861616975/artifacts/3728918754 |
Yugesh-Kumar-S
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Yugesh-Kumar-S - I've reviewed your changes - here's some feedback:
- In SensorChartWidget._buildAxisLabels you have a nested Positioned inside another Positioned—this is redundant and could be simplified to prevent unintended layout behavior.
- There’s a typo in colors.dart (
senosrStatusBorder), please rename it tosensorStatusBorderfor consistency. - BMP180Provider uses a hard-coded data point limit (50) and SensorsScreen has hard-coded sensor lists/addresses—consider extracting these into configurable constants or driving them dynamically to improve maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In SensorChartWidget._buildAxisLabels you have a nested Positioned inside another Positioned—this is redundant and could be simplified to prevent unintended layout behavior.
- There’s a typo in colors.dart (`senosrStatusBorder`), please rename it to `sensorStatusBorder` for consistency.
- BMP180Provider uses a hard-coded data point limit (50) and SensorsScreen has hard-coded sensor lists/addresses—consider extracting these into configurable constants or driving them dynamically to improve maintainability.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Sorry for a very long PR. |
|
@marcnause i have changed the altitude label to estimated altitude(since it is calculated value and not read from sensor) like mr.Bessman suggested in last week meeting. |
|
I was wondering if the axes of the plots should have values like in the old version of the app. I am unsure if this would add valuable information or if it would only be visual clutter. I actually like the clean look of the new version. What do you think? Was it a conscious decision to omit the values? |
Yes sir, I intentionally removed the axis values and added a Current Value section at the top right of the chart. Users can still tap on the chart to view the exact time and sensor values. Since the time interval is in milliseconds, the axis labels were changing too quickly, which made the chart harder to read. |
Fixes #2788
Changes
Screenshots / Recordings
screen-20250802-172807.mp4
Checklist:
strings.xml,dimens.xmlandcolors.xmlwithout hard coding any value.strings.xml,dimens.xmlorcolors.xml.Summary by Sourcery
Port the sensors screen into the Flutter app UI and hook it into the navigation flow.
New Features:
Summary by Sourcery
Port the sensors screens into the Flutter UI and add full support for BMP180 sensor data collection and plotting.
New Features:
Enhancements: