Bug 28669 - Android.Hardware.SensorManager's RegisterListener() APIs have wrong signature
Summary: Android.Hardware.SensorManager's RegisterListener() APIs have wrong signature
Status: ASSIGNED
Alias: None
Product: Android
Classification: Xamarin
Component: BCL Class Libraries ()
Version: 4.20.0
Hardware: Macintosh Mac OS
: Normal normal
Target Milestone: master
Assignee: Atsushi Eno
URL: https://developer.android.com/referen...
Depends on:
Blocks:
 
Reported: 2015-04-01 15:55 UTC by James Athey
Modified: 2017-08-31 05:54 UTC (History)
4 users (show)

Tags: XATriaged
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report for Bug 28669 on Developer Community or GitHub if you have new information to add and do not yet see a matching new report.

If the latest results still closely match this report, you can use the original description:

  • Export the original title and description: Developer Community HTML or GitHub Markdown
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments

Related Links:
Status:
ASSIGNED

Description James Athey 2015-04-01 15:55:45 UTC
All of the RegisterListener() overloads in Android.Hardware.SensorManager that take an ISensorEventListener parameter (as opposed to the deprecated ISensorListener parameter) have the wrong name and type for the "rate" parameter.

According to the Android docs for SensorManager (see the URL in this bug), the third parameter for these overloads is not "int rate" but rather "int samplingPeriodUs", and the "value must be one of SENSOR_DELAY_NORMAL, SENSOR_DELAY_UI, SENSOR_DELAY_GAME, or SENSOR_DELAY_FASTEST or, the desired delay between events in microseconds." Since this parameter can be either an enum OR an arbitrary integer, then the type for this parameter should be int, not Android.Hardware.SensorDelay.

If preserving ABI stability is important, then perhaps 4 new overloads could be created that take int instead of SensorDelay.
Comment 1 Jonathan Pryor 2015-04-02 10:16:57 UTC
> All of the SensorManager.RegisterListener(ISensorEventListener,*) overloads have the
> wrong name...for the "rate" parameter...

Parameter names are problematic, because Google changes them over time:

API-10:
>   public boolean registerListener(SensorEventListener listener, Sensor sensor, int rate);

API-19:
>   public boolean registerListener(SensorEventListener listener, Sensor sensor, int rateUs);

API-21:
>   public boolean registerListener(SensorEventListener listener, Sensor sensor, int samplingPeriodUs);

The parameter name that we use is based on the API level that was bound, because we use the documentation at the time of release of that API level to determine parameter names. (Verifying the above signatures via documentation is problematic unless you know the legacy documentation URLs, so "trust me they changed". As an alternate verification, Xamarin.Android 5.0 contains `class-parse.exe`, which is a Java byte code reader, and the android.jar debug symbols *also* use the above parameter names based on the API level.)

Furthermore, parameter names are per-API level, and thus vary with the $(TargetFrameworkVersion):

> # API-10:
> $ monop -r:/Library/Frameworks/Xamarin.Android.framework/Libraries/xbuild-frameworks/MonoAndroid/v2.3/Mono.Android.dll Android.Hardware.SensorManager | grep RegisterListener
> 	public virtual bool RegisterListener (ISensorEventListener listener, Sensor sensor, SensorDelay rate);

> # API-19:
> $ monop -r:/Library/Frameworks/Xamarin.Android.framework/Libraries/xbuild-frameworks/MonoAndroid/v4.4/Mono.Android.dll Android.Hardware.SensorManager | grep RegisterListener
> 	public virtual bool RegisterListener (ISensorEventListener listener, Sensor sensor, SensorDelay rateUs);

Unfortunately, our binding of API-21 used older documentation, and thus uses the API-19 name instead of the "correct" API-21 name of 'samplingPeriodUs'. When we bind API-22 (Android 5.1), it will get corrected parameter names.

Regardless, parameter names are A Problem™, specifically because of C# named parameters, and it's something we will NOT solve; we're stuck between a rock and a hard place.

If we use the originally introduced parameter name, our documentation and the binding assemblies get "out of sync" with the Android documentation, resulting in the bug you filed. :-)

If we change *all* parameter names to use the parameter names of the latest API level, that will *break* existing source code that uses C# named parameters.

Which leaves us with our current less-than-perfect situation in which parameter names are tied to the API level. Changing $(TargetFrameworkVersion) will thus possibly change parameter names, and thus may result in new compiler errors when named parameters are used.

All solutions are bad in different ways. We are not likely to change the current solution. :-(
Comment 2 Jonathan Pryor 2015-04-02 10:18:04 UTC
> All of the SensorManager.RegisterListener(ISensorEventListener,*) overloads have the
> wrong...type for the "rate" parameter...

This is an oversight from our manual enumification process. You are correct, and we should overload these methods to take `int` in addition to `SensorOverload`.
Comment 3 Jonathan Pryor 2015-04-02 10:21:28 UTC
As a workaround, it is entirely safe and fine to cast arbitrary int values to an enum type:

    manager.RegisterListener (
            listener,
            sensor,
            (SensorDelay) 1000 /* 1s */);
Comment 4 nabeshi 2017-08-31 05:54:32 UTC
I think this parameter is actually 1 millisecond.

    manager.RegisterListener (
            listener,
            sensor,
            (SensorDelay) 1000 /* 1s */); // Is this 1 millisecond?


Excerpted code from SensorManager.java

public boolean registerListener(SensorEventListener listener, Sensor sensor,
        int samplingPeriodUs) {
    return registerListener(listener, sensor, samplingPeriodUs, null);
}

public boolean registerListener(SensorEventListener listener, Sensor sensor,
        int samplingPeriodUs, int maxReportLatencyUs) {
    int delay = getDelay(samplingPeriodUs);
    return registerListenerImpl(listener, sensor, delay, null, maxReportLatencyUs, 0);
}

private static int getDelay(int rate) {
    int delay = -1;
    switch (rate) {
        case SENSOR_DELAY_FASTEST:
            delay = 0;
            break;
        case SENSOR_DELAY_GAME:
            delay = 20000;
            break;
        case SENSOR_DELAY_UI:
            delay = 66667;
            break;
        case SENSOR_DELAY_NORMAL:
            delay = 200000;
            break;
        default:
            delay = rate;
            break;
    }
    return delay;
}

https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/hardware/SensorManager.java#722
https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/hardware/SensorManager.java#779
https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/hardware/SensorManager.java#1878