fix(android): face up or down detection and improve portrait #84
Conversation
WalkthroughThe orientation detection logic was updated by refining pitch and roll tolerance values and introducing new helper methods for more granular face-up and face-down detection. The previous pitch landscape check was replaced with a generic tolerance-based method, and orientation checks now use range-based comparisons instead of exact equality. Changes
Sequence Diagram(s)sequenceDiagram
participant Sensor
participant Utils
participant OrientationClassifier
Sensor->>Utils: Provide pitch and roll values
Utils->>Utils: Check if value is within tolerance (generic method)
Utils->>Utils: Check if roll is close to face-up (new helper)
Utils->>Utils: Check if roll is close to face-down (new helper)
Utils->>OrientationClassifier: Return orientation result
OrientationClassifier->>Sensor: Notify orientation state
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
android/src/main/java/com/orientationdirector/implementation/Utils.kt (2)
29-31: Consider non-zero roll tolerance for better real-world performance.The tighter pitch tolerance and separate face-up/down tolerance are good improvements. However, setting
rollTolerance = 0fmight be too strict for real-world device usage where slight hand movements could cause orientation detection to fail.Consider using a small roll tolerance (e.g., 2f-5f) to account for minor device movements:
- val rollTolerance = 0f + val rollTolerance = 2f
117-123: Improve variable naming for clarity.The detection logic is correct, but the variable name
landscapeLimit = 0fis misleading since this method handles face-up detection, not landscape orientation.Consider renaming for better clarity:
- val landscapeLimit = 0f + val faceUpCenter = 0fOr use the same naming pattern as the face-down method:
- val landscapeLimit = 0f - val faceUpLimit = 10f - - return value in landscapeLimit..faceUpLimit || - value in -faceUpLimit..-landscapeLimit + val faceUpCenter = 0f + val faceUpTolerance = 10f + + return value in faceUpCenter..faceUpTolerance || + value in -faceUpTolerance..faceUpCenter
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/src/main/java/com/orientationdirector/implementation/Utils.kt(3 hunks)
🔇 Additional comments (5)
android/src/main/java/com/orientationdirector/implementation/Utils.kt (5)
43-46: LGTM! Good separation of concerns.The refactoring to use separate pitch checks for landscape vs face-up/down detection with appropriate tolerance values improves code clarity and maintainability.
49-50: Excellent improvement over exact equality checks.The new range-based detection combining roll position with pitch tolerance is much more robust and practical than the previous exact equality checks.
53-53: Good use of constants for consistency.Using
pitchToleranceDefaultinstead of a hardcoded value improves maintainability and consistency across the orientation detection logic.
105-107: Excellent refactoring to a generic utility method.The refactored method is more reusable and follows the DRY principle. The logic correctly checks if a value falls within ±tolerance range.
109-115: Solid implementation of face-down detection.The method correctly identifies face-down orientation by checking for roll values near ±180 degrees with appropriate tolerance. The logic covers both positive and negative ranges properly.
Summary by CodeRabbit
Bug Fixes
Refactor