[Commotion-dev] Quick review of olsr-mesh-tether aka commotion-android

[Hans-Christoph Steiner]

Thanks for the review, please file bug reports in the commotion-android redmine if you want to track the progress on any of these issues.  Comments inline:

On Jun 18, 2012, at 11:45 PM, Jeremy Lakeman wrote:

> The "clients" tab seems like a tether artifact that should be removed.
> Probably other settings that can also go, but that stuck out.

The GUI that's there is just what was inherited from Barnacle WIfi Tether, here's the planned GUI:
https://code.commotionwireless.net/issues/162

> MeshTetherApp.broadcastState should probably be a sticky intent. Which
> would negate the need for the MeshTetherApp.ACTION_CHECK receiver.

Noted, that's a good idea.

> Stopping the app doesn't terminate olsrd, which might cause undefined
> behaviour when upgrading as the previous build of olsrd would be
> restarted.

A known bug due to cramming olsrd into Barnacle.  Will is taking on writing a proper Java daemon wrapper so that should be fixed soon.

> In my case port 8080 was already in use, since you're providing a
> hyperlink in the log the port number doesn't really matter.

The final version will likely only use the jsoninfo plugin, which uses the much less common port 9090.  If port conflicts are still an issue, I think that the best solution is to modify jsoninfo to optionally use a UNIX socket.

> For Serval Mesh we've been considering dropping support for olsr &
> batman and may at some point split out adhoc support to a completely
> separate apk. Partly because we don't want to overload our users with
> info / config which other apps might do better, partly because our
> application can function without adhoc or root access though it is not
> perceived that way.
> 
> I see you have a TOGGLE_STATE broadcast intent we could use, but I was
> wondering if we should try to build a standard interface that's a
> little more generic and allows different implementations to be
> written.
> 
> It should be possible to use the same generic action name in an intent
> filter for a service, yet still allow the calling application to
> specify the implementation to use.
> 
> Any thoughts?


Interesting, so are you thinking that you'll rely on Commotion Mesh Tether to provide Adhoc+OLSR?  This definitely is worth a feature request in redmine.

Following the way this app works, I think the Intents would be TOGGLE_STATE and CHOOSE_PROFILE.  Then we should probably have the option of importing profiles via a JSON file, so that other apps can add the profile they need to MeshTether.

.hc