-
Notifications
You must be signed in to change notification settings - Fork 329
NEW: Add Hinge Angle sensor for Android #1979
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR submission. Code looks good in general, but I struggle to see why we would add this as an Android only API? Wouldn't it be more scalable to add a new sensor type (platform agnostic) with well-defined behavior and definition for what to expect from the angle? Likely also needs a capability check since any implementation trying to utilise this, e.g. reacting to changes in lid angle would need to take into account whether this is available or not if application attempts to support foldable and non-foldable devices.
Packages/com.unity.inputsystem/InputSystem/Plugins/Android/AndroidSensors.cs
Outdated
Show resolved
Hide resolved
/// </summary> | ||
/// <seealso href="https://developer.android.com/reference/android/hardware/Sensor#TYPE_HINGE_ANGLE/> | ||
[InputControlLayout(stateType = typeof(AndroidSensorState), variants = nameof(AndroidSensorType.HingeAngle), hideInUI = true)] | ||
public class AndroidHingeAngle : Sensor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My main concern about this PR is that it adds public platform-specific API surface with no platform-specific particularities? Wouldn't this be applicable for any platform if mapped to existing HID sensors out there also on other platforms? Hinge-sensors have been around for a long time on e.g. laptops.
It's called out in HID Usages standard even though often vendor denied: https://www.usb.org/sites/default/files/hut1_4.pdf, see "23.20.1 Hinge Sensors". Hence I believe there is value to standardise the interface we want for hinge sensor and add it as a non-platform specific sensor type instead if we expand sensor support? I cannot see that this scales. What would happen to this class if Apple adds it or there is iOS or Microsoft APIs added for laptops in your perspective?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what you're basically suggesting is to have a HingeAngle class and AndroidHingeAngle derive from it ?
I can do that of course, didn't consider laptops.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing the update, it seem to make more sense to provide a non-platform specific interface for a sensor as this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update, it also make it even more obvious one has to check for presence of the sensor to actually use it.
Added @IGuscin to help since I don't have access to a folding phone. Also, I see there's some CI failures with it (you can ignore the tvOS ones though) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on most devices currently available.
Galaxy Fold2 (Android 11) and Galaxy Flip2 (Android 11) do not report hinge angle, sensor is not available.
Galaxy Fold3 (Android 11) and Galaxy Flip3 (Android 12) reports 0-90-180 only. After googling a bit - seems to be Samsung/hardware limitation.
Pixel Foldable (Android 14) correctly reports angle at steps of 5 degrees.
Seems to be working fine, haven't seen any issues or drawbacks.
Hey @ekcoh , any ideas what to do about this one https://unity-ci.cds.internal.unity3d.com/job/40124614?utm_source=github ? Should I bump minor version ? |
@todi1856 Yes version should be bumped (minor) on the branch/PR like this that requires it to change. |
…ogies/InputSystem into android/hinge-angle-sensor
I expanded the sample to show to get sensor resolution - 510c6a1 |
Description
Add AndroidHingeAngle sensor, which basically tells how much Android device is unfolded with values ranging from 0 to 180.
Note: There's no foldable Apple phones currently, thus this change is for Android only.
See foldable phone here - https://media.wired.com/photos/6453efc6daed59ebbf4608a7/master/pass/Google-Pixel-Fold-News-Gear.jpg
This change contributes to Unity better supporting large screen & foldable Android devices, as part of contractual obligation with Google - https://jira.unity3d.com/browse/PLAT-9014
Child ticket for tracking https://jira.unity3d.com/browse/PLAT-10298
The change is pretty isolated and doesn't require any change on Android platform backend, since backend always reports both existing and future sensors. So only C# side on input system package was missing.
Changes made
Added public AndroidHingeAngle class, which user can use to access hinge angle sensor and thus get data how much phone is folded.
Testing
Risk
Very low risk, since change is isolated
Checklist
Before review:
Changed
,Fixed
,Added
sections.Area_CanDoX
,Area_CanDoX_EvenIfYIsTheCase
,Area_WhenIDoX_AndYHappens_ThisIsTheResult
.During merge:
NEW: ___
.FIX: ___
.DOCS: ___
.CHANGE: ___
.RELEASE: 1.1.0-preview.3
.After merge: