feat: implementation of the Compass page #2710
Open
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 adds a new Compass instrument page powered by device magnetometer and accelerometer streams, with real-time heading display, selectable ground-parallel axes, and integrates it into the app’s navigation. Class diagram for CompassProvider and CompassScreenclassDiagram
class CompassProvider {
- MagnetometerEvent _magnetometerEvent
- AccelerometerEvent _accelerometerEvent
- StreamSubscription? _magnetometerSubscription
- StreamSubscription? _accelerometerSubscription
- String _selectedAxis
- double _currentDegree
- int _direction
- double _smoothedHeading
+ MagnetometerEvent get magnetometerEvent
+ AccelerometerEvent get accelerometerEvent
+ String get selectedAxis
+ double get currentDegree
+ int get direction
+ double get smoothedHeading
+ void initializeSensors()
+ void disposeSensors()
+ void onAxisSelected(String axis)
+ double getDegreeForAxis(String axis)
}
class CompassScreen {
+ Widget build(BuildContext context)
}
class CompassScreenContent {
+ Widget build(BuildContext context)
}
CompassScreen --> CompassScreenContent
CompassScreenContent --> CompassProvider
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
force-pushed
the
Compass
branch
2 times, most recently
from
8eb04be to
896fb86
Compare
AsCress
requested changes
Jun 2, 2025
|
Build successful. APKs to test: https://github.com/fossasia/pslab-android/actions/runs/16050627243/artifacts/3458076811 |
Made suggested changes
AsCress
requested changes
Jun 3, 2025
AsCress
added
the
Status: Review Required
|
Is this PR still work in progress? |
I’ve tested this on Android, and the compass functionality works well. However, since iOS handles magnetometer data differently, we should test it on an iOS device to ensure consistent behavior and make changes to it accordingly . |
|
@AsCress, can you test this on an iOS device? |
|
@marcnause Unfortunately, currently we can only test on an iOS device after merging the PR. |
|
@Yugesh-Kumar-S Resolve conflicts here. Let's merge this. |
![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:
- The degree display uses
.round().toStringAsFixed(1), but.round()returns an int—either remove the round or call.toStringAsFixed(1)directly on the double to avoid a type mismatch. - Axis selector labels like “X axis” and “Y axis” are hardcoded in the widget—extract them into constants or localization strings to keep your UI text consistent.
- You’re calling
notifyListeners()on every magnetometer and accelerometer event, causing two rebuilds per sensor tick—consider throttling or batching sensor updates to reduce unnecessary rebuilds.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The degree display uses `.round().toStringAsFixed(1)`, but `.round()` returns an int—either remove the round or call `.toStringAsFixed(1)` directly on the double to avoid a type mismatch.
- Axis selector labels like “X axis” and “Y axis” are hardcoded in the widget—extract them into constants or localization strings to keep your UI text consistent.
- You’re calling `notifyListeners()` on every magnetometer and accelerometer event, causing two rebuilds per sensor tick—consider throttling or batching sensor updates to reduce unnecessary rebuilds.
## Individual Comments
### Comment 1
<location> `lib/providers/compass_provider.dart:30` </location>
<code_context>
+ double get smoothedHeading => _smoothedHeading;
+
+ void initializeSensors() {
+ _magnetometerSubscription = magnetometerEventStream().listen(
+ (event) {
+ _magnetometerEvent = event;
+ _updateCompassDirection();
+ notifyListeners();
+ },
+ onError: (error) {
+ logger.e("$magnetometerError: $error");
+ },
+ cancelOnError: true,
+ );
+
</code_context>
<issue_to_address>
Using cancelOnError: true may stop sensor updates permanently after a single error.
Consider handling errors within the stream without setting cancelOnError: true, or implement a mechanism to re-initialize the sensors if the stream is cancelled.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
@AsCress Resolved conflicts and refactored existing hardcoded values. Please merge and test for ios, if it looks good. |
marcnause
approved these changes
Jun 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #2707
Changes
Screenshots / Recordings
Checklist:
strings.xml,dimens.xmlandcolors.xmlwithout hard coding any value.strings.xml,dimens.xmlorcolors.xml.Summary by Sourcery
Add a new Compass instrument page that displays a live, rotatable compass based on magnetometer and accelerometer data with selectable ground-parallel axes.
New Features:
Enhancements: