feat: Implemented Gyroscope #2723
Merged
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
Reviewer's GuideThis PR completes the Gyroscope feature by mirroring the existing Accelerometer implementation: it adds navigation support, string constants, a dedicated provider, chart widget, and a screen tied together with routing. Sequence Diagram: Navigating to Gyroscope ScreensequenceDiagram
participant U as User
participant IS as InstrumentsScreen
participant Nav as Navigator
participant GS as GyroscopeScreen
U->>IS: Selects Gyroscope (item 10)
IS->>Nav: pushNamedAndRemoveUntil("/gyroscope", (route) => route.isFirst)
Nav->>GS: Creates and displays GyroscopeScreen
Sequence Diagram: Gyroscope Data FlowsequenceDiagram
participant GScreen as GyroscopeScreen
participant GP as GyroscopeProvider
participant Sensor as GyroscopeSensor (sensors_plus)
participant GCard as GyroscopeCard
GScreen->>GP: Create / Initialize (GyroscopeProvider()..initializeSensors())
activate GP
GP->>Sensor: gyroscopeEventStream().listen()
deactivate GP
loop Data Stream
Sensor-->>GP: GyroscopeEvent
activate GP
GP->>GP: _updateData()
GP->>GScreen: notifyListeners() // Triggers UI update
deactivate GP
end
GScreen->>GScreen: Rebuild UI
GScreen->>GCard: Update with new data from GyroscopeProvider
GCard->>GCard: Display data in chart
Class Diagram: New Gyroscope ComponentsclassDiagram
class GyroscopeScreen {
+build(BuildContext context) Widget
}
class _GyroscopeScreenState {
+build(BuildContext context) Widget
}
class GyroscopeCard {
+String axis
+Color color
+createState() State
}
class _GyroscopeCardState {
+build(BuildContext context) Widget
+sideTitleWidgets(double value, TitleMeta meta) Widget
}
class GyroscopeProvider {
<<ChangeNotifier>>
-StreamSubscription_GyroscopeEvent_ _gyroscopeSubscription
-GyroscopeEvent _gyroscopeEvent
-List_double_ _xData
-List_double_ _yData
-List_double_ _zData
+List_FlSpot_ xData
+List_FlSpot_ yData
+List_FlSpot_ zData
+double xValue
+double yValue
+double zValue
+double xMin
+double xMax
+double yMin
+double yMax
+double zMin
+double zMax
+bool isListening
+initializeSensors() void
+disposeSensors() void
-_updateData() void
+getAxisData(String axis) List_FlSpot_
+getMin(String axis) double
+getMax(String axis) double
+getCurrent(String axis) double
+getDataLength(String axis) int
+dispose() void
}
class GyroscopeEvent {
<<external: sensors_plus>>
+double x
+double y
+double z
+DateTime timestamp
}
class FlSpot{
<<external: fl_chart>>
+double x
+double y
}
GyroscopeScreen "1" --o "1" _GyroscopeScreenState : creates
_GyroscopeScreenState ..> GyroscopeProvider : uses (via Provider)
_GyroscopeScreenState "1" *-- "3" GyroscopeCard : displays
GyroscopeCard "1" --o "1" _GyroscopeCardState : creates
_GyroscopeCardState ..> GyroscopeProvider : uses (via Provider)
GyroscopeProvider ..> GyroscopeEvent : uses
GyroscopeProvider ..> FlSpot : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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:
- Eliminate the redundant notifyListeners() call since it’s already being invoked in _updateData(), to avoid double rebuilds.
- Fix the typo in constants.dart ('No data availabl' should read 'No data available').
- Initialize your min/max trackers (e.g. with ±double.infinity or the first data point) instead of zero so the computed minima/maxima are accurate from the start.
Here's what I looked at during the review
- 🟡 General issues: 6 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+44
to
+45
| _updateData(); | ||
| notifyListeners(); |
There was a problem hiding this comment.
issue: Duplicate notifyListeners call
Consider removing one of the notifyListeners calls to prevent unnecessary rebuilds.
Comment on lines
+38
to
+42
| double currVal = provider.getCurrent(widget.axis.toLowerCase()); | ||
| double minVal = provider.getMin(widget.axis.toLowerCase()); | ||
| double maxVal = provider.getMax(widget.axis.toLowerCase()); | ||
| int dataLength = provider.getDataLength(widget.axis.toLowerCase()); | ||
|
|
There was a problem hiding this comment.
suggestion: Use dynamic Y-axis bounds
Hard-coding these values risks clipping actual data. Compute chartMinY and chartMaxY dynamically from the provider's min and max, adding appropriate padding.
Suggested change
| double currVal = provider.getCurrent(widget.axis.toLowerCase()); | |
| double minVal = provider.getMin(widget.axis.toLowerCase()); | |
| double maxVal = provider.getMax(widget.axis.toLowerCase()); | |
| int dataLength = provider.getDataLength(widget.axis.toLowerCase()); | |
| double currVal = provider.getCurrent(widget.axis.toLowerCase()); | |
| double minVal = provider.getMin(widget.axis.toLowerCase()); | |
| double maxVal = provider.getMax(widget.axis.toLowerCase()); | |
| int dataLength = provider.getDataLength(widget.axis.toLowerCase()); | |
| // Compute dynamic Y-axis bounds with padding | |
| double yPadding = (maxVal - minVal).abs() * 0.1; | |
| if (yPadding == 0) { | |
| yPadding = 1.0; // fallback padding if min and max are equal | |
| } | |
| double chartMinY = minVal - yPadding; | |
| double chartMaxY = maxVal + yPadding; |
| Widget build(BuildContext context) { | ||
| GyroscopeProvider provider = Provider.of<GyroscopeProvider>(context); | ||
|
|
||
| List<FlSpot> spots = provider.getAxisData(widget.axis.toLowerCase()); |
There was a problem hiding this comment.
suggestion: Handle empty data scenario in UI
Consider displaying a placeholder or the noData constant when spots is empty to ensure proper chart rendering.
| void initializeSensors() { | ||
| if (_gyroscopeSubscription != null) return; | ||
|
|
||
| _gyroscopeSubscription = gyroscopeEventStream().listen( |
There was a problem hiding this comment.
suggestion (performance): Throttle sensor update frequency
Consider throttling or sampling the sensor event stream to reduce unnecessary listener rebuilds and improve performance.
Suggested implementation:
// Add import at the top of the file:
// import 'package:rxdart/rxdart.dart';
void initializeSensors() {
if (_gyroscopeSubscription != null) return;
_gyroscopeSubscription = gyroscopeEventStream()
.throttleTime(const Duration(milliseconds: 50))
.listen(
(event) {
_gyroscopeEvent = event;
_updateData();
notifyListeners();
},
onError: (error) {
logger.e("Gyroscope error: $error");
},
cancelOnError: true,
);
}
- Add
import 'package:rxdart/rxdart.dart';at the top of the file if not already present. - Ensure
rxdartis included in yourpubspec.yamldependencies. - Adjust the throttle duration as needed for your use case.
|
Build successful. APKs to test: https://github.com/fossasia/pslab-android/actions/runs/15447328389/artifacts/3260712295 |
|
@Yugesh-Kumar-S do you think it is wise to consider the suggestions by Sourcery? |
Yes sir , some of the Sourcery suggestions do improve the performance . To maintain code consistency i have implemented it similar to the Accelerometer , but the suggestions can also be included. |
|
@CloudyPadmal Seems mostly nice to me. The performance is good, I've tested on both Android and iOS. |
CloudyPadmal
approved these changes
Jun 5, 2025
Yugesh-Kumar-S
added a commit
to Yugesh-Kumar-S/pslab-android
that referenced
this pull request
Jun 7, 2025
# This is the 1st commit message: Reimplemented a simple themeing # This is the commit message #2: #feat: Implemented Gyroscope (fossasia#2723) #resolved conflicts
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Fixes #2715
Changes
Screenshots / Recordings
Checklist:
strings.xml,dimens.xmlandcolors.xmlwithout hard coding any value.strings.xml,dimens.xmlorcolors.xml.Summary by Sourcery
Implement a dedicated gyroscope instrument screen mirroring the existing accelerometer view to display real-time X, Y, and Z rotational data.
New Features:
Enhancements:
Chores: