Skip to content

Conversation

todi1856
Copy link
Member

@todi1856 todi1856 commented Aug 7, 2024

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

  • Add synthetic tests
  • Manually tested on Google Fold.

Risk

Very low risk, since change is isolated

Checklist

Before review:

  • Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
    • Jira port for the next release set as "Resolved".
  • Tests added/changed, if applicable.
    • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration tests.
  • Docs for new/changed API's.
    • Xmldoc cross references are set correctly.
    • Added explanation how the API works.
    • Usage code examples added.
    • The manual is updated, if needed.

During merge:

  • Commit message for squash-merge is prefixed with one of the list:
    • NEW: ___.
    • FIX: ___.
    • DOCS: ___.
    • CHANGE: ___.
    • RELEASE: 1.1.0-preview.3.

After merge:

  • Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

@todi1856 todi1856 changed the title NEW Add Hinge Angle sensor for Android NEW: Add Hinge Angle sensor for Android Aug 7, 2024
Copy link
Collaborator

@ekcoh ekcoh left a 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.

/// </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
Copy link
Collaborator

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?

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

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.

@todi1856 todi1856 requested a review from ekcoh August 8, 2024 09:29
Copy link
Collaborator

@ekcoh ekcoh left a 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.

@ekcoh ekcoh requested a review from Pauliusd01 August 9, 2024 08:35
@Pauliusd01 Pauliusd01 requested a review from IGuscin August 9, 2024 09:55
@Pauliusd01
Copy link
Collaborator

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)

Copy link

@IGuscin IGuscin left a 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.

@Pauliusd01 Pauliusd01 removed their request for review August 9, 2024 11:15
@todi1856
Copy link
Member Author

todi1856 commented Aug 9, 2024

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 ?

@ekcoh
Copy link
Collaborator

ekcoh commented Aug 12, 2024

@todi1856 Yes version should be bumped (minor) on the branch/PR like this that requires it to change.

@ekcoh
Copy link
Collaborator

ekcoh commented Aug 12, 2024

@IGuscin @todi1856 Maybe this indicates that there is need to know device dependent resolution of the sensor value? Maybe we should state in the sensor API docs that the resolution of the sensor value is device/implementation dependent at least?

@todi1856
Copy link
Member Author

@IGuscin @todi1856 Maybe this indicates that there is need to know device dependent resolution of the sensor value? Maybe we should state in the sensor API docs that the resolution of the sensor value is device/implementation dependent at least?

I expanded the sample to show to get sensor resolution - 510c6a1

@ekcoh ekcoh merged commit 445db66 into develop Aug 13, 2024
@ekcoh ekcoh deleted the android/hinge-angle-sensor branch August 13, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants