Implemented a basic json logger #9

jliebers · 2021-02-03T20:54:18Z

first of all, thank you for your work with this fantastic app. I took a quick look at your thesis and I have to tell that we share the same goal. In my PhD I focus on getting rid of passwords as well. And for that reason I intend to log some spatial movement through an Android Wear Device (Moto 360 in my case), but I need that data logged to file for a post-hoc analysis. Whilst your app already does an excellent job of visualization, connection etc., I am glad to add this missing feature (#8). 🙂 🚀

Please find a pull request for some basic logging functionality that adds a second floating action button to start and stop the logging of the selected sensors to a JSON file. The file then is stored in Android's external storage in a directory resembling the package name. I also added some example data. It is only a very basic implementation, so please do not judge me (I did some Android in the past, but that's quite some years ago), but I think it does the job and it might be better than nothing. Also I tried to comment/document the most important things in the code.

If you have any comments I am open for feedback. I would be very happy if you could review the changes and in case of approval, please push an update to Google Play.

On a side note, I also had some troubles getting Gradle and the wear-build to work. Reason lies within some wear-library on version 1.4.0 being removed from the mirrors. So I had to fetch the files from an unofficial mirror and place them at this location: %appdata%\..\Local\Android\Sdk\extras\m2repository\com\google\android\support\wearable\1.4.0 (Win10). After that I built the app.

Everything was tested on a Sony Xperia XZ2 Compact and a Moto 360 smart watch.

Cheers!

… list

Steppschuh · 2021-02-04T09:19:35Z

Hey Jonathan, thanks for your contribution! I'll look into this on the weekend and get back to you.

Regarding the deprecated wearable dependency: Are there breaking changes when updating to a newer version? If not, it might be easier to just migrate to a more recent version (same applies to other dependencies).

jliebers · 2021-02-04T18:23:59Z

Hey Stephan,

Regarding the deprecated wearable dependency: Are there breaking changes when updating to a newer version? If not, it might be easier to just migrate to a more recent version (same applies to other dependencies).

I tried to update the libraries but to be honest I am not an expert on Gradle or the Android Build ecosystem. So I naturally ran into lots of problems and gave up after ca. 2 hours of trying. Instead I opted to run the old build as described. But most likely someone with more experience can sort it out easily ... or not (as I cannot know). 😉

Cheers,

Jonathan

Steppschuh

I've added a few suggestions. Please don't feel obliged to implement them all, it's just what came to my mind looking at the PR.

Again, thanks for your contribution!

Steppschuh · 2021-02-06T09:57:50Z

Android Source/mobile/src/main/java/net/steppschuh/sensordatalogger/PhoneActivity.java

+                if (ContextCompat.checkSelfPermission(getApplicationContext(),
+                        Manifest.permission.WRITE_EXTERNAL_STORAGE)
+                        != PackageManager.PERMISSION_GRANTED) {
+                    if (ActivityCompat.shouldShowRequestPermissionRationale(app.getContextActivity(),


Negate this to remove the empty if branch or actually show a rationale

Removed the empty branch by removing the permission handling. 👍

Steppschuh · 2021-02-06T10:03:42Z

Android Source/mobile/src/main/java/net/steppschuh/sensordatalogger/PhoneActivity.java

+            @Override
+            public void onClick(View v) {
+                // request runtime permission
+                if (ContextCompat.checkSelfPermission(getApplicationContext(),


As the minimum SDK version is 21, you can probably avoid requesting this permission. According to the documentation:

Starting in API level 19, this permission is not required to read/write files in your application-specific directories returned by Context.getExternalFilesDir(String)

(1/3) Removed the permission checking, good to know that it was not required. 👍

Steppschuh · 2021-02-06T10:10:46Z

Android Source/mobile/src/main/java/net/steppschuh/sensordatalogger/PhoneActivity.java

+                }
+
+                // if user did not grant permission, notify by Toast
+                if (ContextCompat.checkSelfPermission(getApplicationContext(),


Not sure why this is checked again here. Note that requesting permissions is an asynchronous process, all permission request results will be delivered via a callback method that you have to override (onRequestPermissionsResult). See requesting runtime permissions.

(2/3) Removed the permission checking, good to know that it was not required. 👍

Steppschuh · 2021-02-06T10:13:12Z

Android Source/mobile/src/main/java/net/steppschuh/sensordatalogger/PhoneActivity.java

+                    if (ActivityCompat.shouldShowRequestPermissionRationale(app.getContextActivity(),
+                            Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
+                    } else {
+                        ActivityCompat.requestPermissions(app.getContextActivity(),


You're currently in the PhoneActivity, so every time you need a Context you can simply use this instead of getApplicationContext() or app.getContextActivity().

This did not work for me as this points to the current instance of OnClickListener.

Steppschuh · 2021-02-06T10:18:02Z

Android Source/mobile/src/main/java/net/steppschuh/sensordatalogger/PhoneActivity.java

+                            Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
+                    } else {
+                        ActivityCompat.requestPermissions(app.getContextActivity(),
+                                new String[]{Manifest.permission.WRITE_EXTERNAL_STORAGE},23


The 23 is the request code that you would get back in the result callback. Although the value itself doesn't matter too much, this looks like a magic number. Usually you would define a constant for that, like private static final int STORAGE_PERMISSION_REQUEST_CODE = 1;

(3/3) Removed the permission checking, good to know that it was not required. 👍

Steppschuh · 2021-02-06T10:35:50Z

Android Source/mobile/src/main/java/de/jo2k/loggingmodule/DataHandler.java

+                filewriter.write(",");
+            }
+            firstEntryWasWritten = true;
+            filewriter.write(dataBatch.toJson());


Writing every DataBatch to a file as soon as its received sounds like a very big overhead. The file will bloat up a lot because of the JSON format and IO operations are quite slow and will block the current thread.

I would recommend collecting multiple of these data batches, merging them together and then flushing them to the file writer occasionally (and when stopping the recording).

I have implemented a Queue that is read by a runnable (thread) in the background and flushes the contents to file. 👍

Steppschuh · 2021-02-06T10:38:07Z

Android Source/mobile/src/main/java/de/jo2k/loggingmodule/DataHandler.java

+        this.filewriter = filewriter;
+    }
+
+    public String stopRecording () {


Similar to startRecording, I would return the created File here. Or make it void and add a getter for the most recently created file.

👍 (Included both suggestions)

Steppschuh · 2021-02-06T10:43:59Z

Android Source/mobile/src/main/java/de/jo2k/loggingmodule/DataHandler.java

+    private boolean firstEntryWasWritten;
+
+    public void logDataBatch(DataBatch dataBatch, String sourceNodeId) {
+        /**


This style of block comment (used for method documentation) needs to be placed right above the method, not in the method body.

Got it, wrote to much Python in the past apparently where it is flipped around. 👍

Steppschuh · 2021-02-06T10:48:11Z

Android Source/mobile/src/main/java/net/steppschuh/sensordatalogger/PhoneActivity.java

+                // start a new recording if no recording is going on.
+                // pass current timestamp as a filename and raise a Toast to notify user of start
+                // and stop action. Also assign a new icon to the second floating action button.
+                if (! dataHandler.isRecording()) {


Suggested change

if (! dataHandler.isRecording()) {

if (!dataHandler.isRecording()) {

Steppschuh · 2021-02-06T10:51:51Z

Android Source/mobile/src/main/java/de/jo2k/loggingmodule/DataHandler.java

+    public String stopRecording () {
+        // terminate early if recording was not started beforehand
+        // (should not happen.)
+        if (! isRecording)


Suggested change

if (! isRecording)

if (!isRecording)

jliebers · 2021-02-10T09:21:34Z

Hey,

thanks for the feedback, I find it valuable and I will incorporate it once I find the time (probably around the weekend). Thanks!

Cheers,

Jonathan

jliebers · 2021-02-19T19:07:54Z

Hi, thanks for the feedback. I have included all of it and pushed a new commit to my branch. 🙂 If you like, take a look. Thanks in advance.

PS: I have tested the new implementation on a LG LM-K510 with a Moto 360.

jliebers added 5 commits February 3, 2021 20:45

Implemented basic json logger

0ddb7d6

Added missing import and implemented correct comma seperation of json…

5bcb6d0

… list

Bumping all version numbers to 1.7

6ba9d44

Added example data to repo

06e1d13

Added some more details to README.md

662eab7

Fixed a wrong string in string.xml (renamed CSV to JSON)

c1df33c

Steppschuh requested changes Feb 6, 2021

View reviewed changes

Included all feedback.

f41e43a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented a basic json logger #9

Implemented a basic json logger #9

jliebers commented Feb 3, 2021 •

edited

Steppschuh commented Feb 4, 2021

jliebers commented Feb 4, 2021

Steppschuh left a comment

Steppschuh Feb 6, 2021

jliebers Feb 19, 2021

Steppschuh Feb 6, 2021

jliebers Feb 19, 2021

Steppschuh Feb 6, 2021

jliebers Feb 19, 2021

Steppschuh Feb 6, 2021

jliebers Feb 19, 2021

Steppschuh Feb 6, 2021

jliebers Feb 19, 2021

Steppschuh Feb 6, 2021

jliebers Feb 19, 2021

Steppschuh Feb 6, 2021

jliebers Feb 19, 2021

Steppschuh Feb 6, 2021

jliebers Feb 19, 2021

Steppschuh Feb 6, 2021

jliebers Feb 19, 2021

Steppschuh Feb 6, 2021

jliebers Feb 19, 2021

jliebers commented Feb 10, 2021

jliebers commented Feb 19, 2021 •

edited

	if (! dataHandler.isRecording()) {
	if (!dataHandler.isRecording()) {

Implemented a basic json logger #9

Are you sure you want to change the base?

Implemented a basic json logger #9

Conversation

jliebers commented Feb 3, 2021 • edited

Steppschuh commented Feb 4, 2021

jliebers commented Feb 4, 2021

Steppschuh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jliebers commented Feb 10, 2021

jliebers commented Feb 19, 2021 • edited

jliebers commented Feb 3, 2021 •

edited

jliebers commented Feb 19, 2021 •

edited