Support creating Observable from Android's Sensor.

[amazari]

Very early PR to get some guidance over API, testing, style and error handling.

[cloudbees-pull-request-builder]

RxJava-pull-requests #476 SUCCESS
This pull request looks good

[benjchristensen]

This needs to be rebased as it conflicts with other changes committed.

@mttkay Can you weigh in on this?

[zsxwing]

We also need to use unregisterListener to remove this listener. You can check how to do it in swing: https://github.com/Netflix/RxJava/blob/master/rxjava-contrib/rxjava-swing/src/main/java/rx/swing/sources/MouseEventSource.java

[mttkay]

Yeah I was thinking that. Should return a Subscription that closes the subject and removes the internal listener.

Would be good to have unit tests too. Have a look at the existing Android specific classes to see how we use Robolectric to test them.

I'm also getting a weird compilation error in IntelliJ (it says PublishSubject<float[]> is not a subtype of Observable<float[]>, but it builds fine using Gradle, so that might be an issue with my IDE setup?

Apart from that looks good to me. @benjchristensen I was wondering though, in what way does this not merge cleanly? I don't see with what this would conflict and GH isn't reporting merge conflicts for me.

[headinthebox]

You should not need a subject. Just Observable.create (whenever you think you need a subject, think again).

To handle @mironov-nsk remark, I would pass in a function from SensorEvent to T, you use that function to project the event to whatever you want to push into the stream.

public static Observable<float[]> fromSensor(SensorManager sensorManager, int type, int rate) {}

would become

 public static <T> Observable<T> fromSensor(SensorManager sensorManager, int type, int rate, Func1<SensorEvent, T> selector) {}

[zsxwing]

@mttkay When a user subscribes an Observable, he may forget to unsubscribe it which may be a resource leak issue in Android. I propose the following helper class to help users manage the Subscriptions related to a Context.
So the user can simply unsubscribe all of the Subscriptions at the onStop method.

public class AndroidSubscriptions {

    private static Map<Context, CompositeSubscription> contextSubscriptions = new HashMap<Context, CompositeSubscription>();

    public static synchronized void add(Context context,
            Subscription subscription) {
        CompositeSubscription parentSubscription = contextSubscriptions
                .get(context);
        if (parentSubscription == null) {
            parentSubscription = new CompositeSubscription();
            contextSubscriptions.put(context, parentSubscription);
        }
        parentSubscription.add(subscription);
    }

    public static synchronized void unsubscribe(Context context) {
        CompositeSubscription parentSubscription = contextSubscriptions
                .get(context);
        if (parentSubscription != null) {
            parentSubscription.unsubscribe();
            contextSubscriptions.remove(context);
        }
    }
}

Here I use static methods because I want to use AndroidSubscriptions.add like the following codes:

public class OperationObserveFromAndroidSensor {

    public static Observable<float[]> observeFromAndroidSensor(
            final Context context, final SensorManager sensorManager, int type,
            final int rate) {
        final Sensor sensor = sensorManager.getDefaultSensor(type);
        if (sensor == null)
            throw new IllegalArgumentException("Unsupported sensor type.");

        return Observable.create(new OnSubscribeFunc<float[]>() {

            @Override
            public Subscription onSubscribe(
                    final Observer<? super float[]> observer) {
                final SensorEventListener listener = new SensorEventListener() {
                    @Override
                    public void onSensorChanged(SensorEvent event) {
                        observer.onNext(Arrays.copyOf(event.values,
                                event.values.length));
                    }

                    @Override
                    public void onAccuracyChanged(Sensor sensor, int accuracy) {
                        // TODO
                    }
                };
                sensorManager.registerListener(listener, sensor, rate);
                Subscription subscription = Subscriptions.create(new Action0() {

                    @Override
                    public void call() {
                        sensorManager.unregisterListener(listener);

                    }
                });
                AndroidSubscriptions.add(context, subscription);
                return subscription;
            }
        });
    }
}

What's your opinion?

[mttkay]

I'm not sure I understand what problem this solves. Where do you release the composite subscription for a context? The wrapped subscriptions, while mapped to the same context, might have completely unrelated life cycles too. Moreover, keeping strong references to Context in a global, unsynchronized shared object is a bit of a red flag.

[zsxwing]

The composite subscription for a context will be unsubscribed in Activity.onStop (or onDestroy?) method, like:

    @Override
    protected void onStop() {
        AndroidSubscriptions.unsubscribe(this);
        super.onStop();
    }

[zsxwing]

Basically, I want to collect all of the subscriptions related to a context. When the context is going to be destroyed, we can call AndroidSubscriptions.unsubscribe(context) to subscribe them.

[mttkay]

I see what you mean. Why not have the composite subscription as a member
field in your Activity class? That's what we do in our app. I still don't
see a case here for a shared static field in the library itself.

On Thu, Dec 12, 2013 at 10:20 AM, Shixiong Zhu [email protected]:

Basically, I want to collect all of the subscriptions related to a
context. When the context is going to be destroyed, we can call
AndroidSubscriptions.unsubscribe(context) to subscribe them.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/543#issuecomment-30400740
.

[zsxwing]

@mttkay You're right. I gave up this idea as I could not find any elegant way. I agree that managing the subscriptions is the app's job.

[benjchristensen]

Since this can no longer be merged, can you close this and re-create a new one once the code is ready?

[benjchristensen]

Closing out as it can't be merged and has been 15+ days since involvement ... please re-submit a new PR when this is ready.