Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making it work on devices without sdcards #32

Closed
wants to merge 3 commits into from

Conversation

rom1v
Copy link
Contributor

@rom1v rom1v commented Oct 4, 2012

I tried to use a device without sdcard, and the application crashes.

The first commit (c5923b3) essentially allows to store serval-mesh.db in internal storage if there is no sdcard.

But after that, rhizome still tries to use /sdcard storage. We could modify its path in serval.conf, but we can also disable rhizome. Such an option was partially implemented (adding "rhizome.enable=0" in serval.conf), but 2 things were missing:

  1. servald start opened rhizome database, even if it was disabled ;
  2. client called rhizome functions, even if it was disabled.

The second commit (efc94c5) fixes a part of 2).
Two commits in serval-dna (ad3b316 and c31c1fe) fix 1) and the rest of 2).

After these changes, I successfully phoned another device (without root access, with a wifi router) on a Motorola Milestone Android 2.2, having no sdcard (disabling rhizome).

Regards,
®om

Batphone should be able to run on a device without sdcard.

Use ServalBatPhoneApplication.getStorageFolder() for retrieving the sdcard
folder if it is present, an internal storage folder otherwise.

This is necessary but not sufficient, because Rhizome *needs* sdcard.
Rhizome *needs* sdcard. If no sdcard is present on the device, then rhizome must
not be called.

A feature to disable it was already implemented, just add:
  rhizome.enable=0
to serval.conf (don't forget '\n').

But it was not read by the Android application not to call rhizome functions.

This commit disable rhizome calls and avoids to make the app crash when no
sdcard is present. It needs the modifications done on 'serval-dna' submodule.
@gardners
Copy link
Member

gardners commented Oct 5, 2012

Hi,
Thanks for attacking this. It should be possible to also enable Rhizome using internal storage only for people without an SDcard in their phone. From what I can see after your commits all we need to do is tell serval-dna that it can put the rhizome database in a different location if there is no sdcard.
We will finish reviewing your changes, and merge them in all going well (but I don't expect any problems).
Paul.

@lakeman
Copy link
Member

lakeman commented Oct 5, 2012

So running your new "servald rhizome enabled" is equivalent to "servald config get rhizome.enabled"?
Do we need a separate command for this?

I don't think getStorageFolder() should ever return "/BatPhone".
Let's just get rid of all references to that folder and change getStorageFolder() to always return [external]/serval or [DATA_FILE_PATH]/var.
SetPhoneNumber reads from there for people who are upgrading from an ancient version by uninstalling and reinstalling. Which is mainly developers, and even then developers don't usually need to uninstall.
Lets just drop that now.

I'm tempted to drop this "feature" of remembering your phone number between installs anyway, it's not that time consuming to enter a new number and we aren't backing up your private key so you are effectively a different person as it is.

Otherwise this is pretty good.

@rom1v
Copy link
Contributor Author

rom1v commented Oct 5, 2012

@gardners
Yes, enabling Rhizome even without sdcard would be great.

@lakeman
I am very new to serval project (I didn't know it 4 days ago, it is very interesting). I didn't know there was another command, which of course we should use instead ("config get rhizome.enable").

Just a tiny problem: it returns the config value "as it is" in serval.conf. For boolean values, you can indifferently use {"on", "yes", "true", "1"} (resp. {"off", "no", "false", "0"}).
Should we duplicate conf.c/confParseBoolean() in Java for boolean-parsing the String ?
Or should we "normalize" the value before returning it (in commandline.c/app_config_get() for example) ?

For getStorageFolder(), I tried to make as few changes as possible: I didn't know why sometimes data were written in /BatPhone, but sometimes not, so I kept it.
Now you say we should never use /BatPhone, I changed it (just committed 4ba7389).
However, I let you change SetPhoneNumber.

®om

@gardners
Copy link
Member

gardners commented Oct 5, 2012

Hello,

On Fri, Oct 5, 2012 at 3:22 PM, Jeremy Lakeman notifications@github.comwrote:

So running your new "servald rhizome enabled" is equivalent to "servald
config get rhizome.enabled"?
Do we need a separate command for this?

My gut feeling is that the existing command is sufficient, provided that
its output is readily parseable by the java side, which I think is the
case. If not, we can fix it.

I don't think getStorageFolder() should ever return "/BatPhone".
Let's just get rid of all references to that folder and change
getStorageFolder() to always return [external]/serval or
[DATA_FILE_PATH]/var.

Agreed.

SetPhoneNumber reads from there for people who are upgrading from an
ancient version by uninstalling and reinstalling. Which is mainly
developers, and even then developers don't usually need to uninstall.
Lets just drop that now.

I'm tempted to drop this "feature" of remembering your phone number
between installs anyway, it's not that time consuming to enter a new number
and we aren't backing up your private key so you are effectively a
different person as it is.

Also agreed.

Otherwise this is pretty good.

Okay. Let's work on those changes and merging it in when done.

Paul.


Reply to this email directly or view it on GitHubhttps://github.com//pull/32#issuecomment-9166193.

@lakeman
Copy link
Member

lakeman commented Oct 5, 2012

I've pulled your changes into my repo, with some slight changes. I'll push them manually once I've also tidied up some other issues I mentioned above and had a chance to re-test.

@lakeman
Copy link
Member

lakeman commented Oct 8, 2012

Most changes have now been applied manually. With some further path related changes built on top.

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.

None yet

3 participants