Make the LocalBinder class public

[rking788]

Made the LocalBinder class and getService() method public so they can be used outside of RosActivity.

Fixes #238

[stratomda]

Rking, Do you know what is happening with this pull request? I am trying to implement a RosCompatActivity and need the LocalBinder public. Have you found a workaround?

[rking788]

@stratomda I think this pull request is still pending.

Right now I am just maintaining a local build of the android_core AAR file with the binder public.

EDIT: it is nice to see that this would benefit someone other than myself.

[stratomda]

@Rking788 I am trying to build a copy of my modified android_10 aar file (see this link) any thoughts? Thanks,

[stratomda]

@adamantivm when I get my kinetic workspace up can we merge a similar pull request for this issue? This allows for implementation of a custom MasterChooser.

[adamantivm]

Yes. Sorry I still couldn't get around to reviewing older pending pull requests such as this one. It is on my bucket list. Working on top of kinetic would be idea, would make things easier. I do think we should go back to indigo and contributions like this though.

/cc @jubeira @ernestmc it would be interested to know if you have any opinion about custom master choosers.

[stratomda]

@adamantivm @jubeira @ernestmc if a custom master chooser isn't the way to go, we should revamp the current one. Here are a few things I implemented in my own:

1. Catching exception when master URI is a valid IP but no roscore is running. Currently in indigo the application crashes

2. Catch malformed URI

3. UI design update to include progress spinner when attempting connection

4. AppCompatActivity

Thoughts?

[adamantivm]

I agree with all those requirements. At a higher level I think both a custom master chooser AND an updated default one would be in order. I would add for a default one the ability to work with a preferences activity, default values and automatic start (if the configuration is already set).

[jubeira]

I agree with the comments above. I think that catching exceptions on valid IPs with no master running and malformed URIs / unreachable IP addresses is specially important. For a custom master chooser, the current way of implementing it would be overriding startMasterChooser from RosActivity. I'm not sure if it is worth modifying RosActivity to change this behavior, but perhaps there is a better way of doing this.

[stratomda]

We could also provide it to the RosActivity through an overloaded constructor. Then in the startMasterChooser check for the local custom chooser, otherwise use the default. Then the user doesn't have to override the whole startMasterChooser (prevent them from messing up the precondition on the startActivityForResult with an incorrect request code).

[jubeira]

@stratomda I agree with that approach. One motivation to have a custom master chooser is to integrate it with a settings activity containing other settings (just like in this application).

Talking with @adamantivm, we came up with an idea for the possible solution:

• Add a constructor receiving an intent for the activity to launch, and a custom request code.
• Launch the custom activity in startMasterChooser using the custom request code (the current MasterChooser will be launched with the default constructor).
• Finally, the code inside onActivityResult would be placed inside a callback that could be set with a public method. The default behavior wouldn't be modified, but the user could choose to perform different actions and even handle different results or request codes.

Any comments about this? I will open an issue (or two) summarizing this discussion. Your improvements for a revamped MasterChooser sound very good, adding the option to start directly with a previously configured URI. And for that, catching connection errors is required.

[gautamjain]

Will this still be merged?

I'm trying to start NodeMainExecutorService from another Service and then use it to execute a few NodeMains. Making the LocalBinder class and getService() method public would make this possible.

[jubeira]

@adamantivm I tend to agree with this change. If you don't have any strong opinions against it, I'd merge it to Indigo; I offer myself to cherry-pick the commit and apply it to Kinetic as well. /cc @ernestmc

[adamantivm]

I trust your judgement @jubeira , go ahead ;)