Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

export fetched data into a csv file #777

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Vebryn
Copy link
Contributor

@Vebryn Vebryn commented Aug 23, 2017

Hi,

Here is a clean version of csv export implementation. Old conversation can be found into #692.

Best regards

Copy link
Contributor

@cpfeiffer cpfeiffer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, nice!

Can you explain why added two CSV export implementations? One in the SampleProvider and the other in the CSVExport class? I think one should be sufficient :-)


LOG.info("Exporting samples into csv file: " + outFile.getName());
try {
BufferedWriter bw = new BufferedWriter(new FileWriter(outFile));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use
try (BufferedWriter bw = ....) {
and remove the bw.close()
That way the writer will always be closed (also when an exception occurs).

Copy link
Contributor Author

@Vebryn Vebryn Sep 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know this try block syntax. So, it's a try-with-resources statement. It closes bw at end of execution (cf. https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html). I understand so why you remove bw.close().

bw.flush();
bw.close();
} catch (IOException e) {
LOG.error(e.getMessage());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use
LOG.error("Explain what failed", e);
otherwise, the stacktrace of the exception will not be logged, making debugging hard.

// export csv on fetching
File myPath = null;
myPath = FileUtils.getExternalFilesDir();
File myFile = new File(myPath, startTimestamp.getTimeInMillis() / 1000 + ".csv");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a gadgetbridgedb_ prefix (or something like that, maybe shorter)?

But really, this does not belong here. It should be implemented in one place for all devices, not in the Mi Band 2 FetchActivityOperation. And I guess we should also have an option to turn this on and off.

Copy link
Contributor Author

@Vebryn Vebryn Sep 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, does data from other devices differs ? Where would you place on/off feature ? I would probably add into Mi Band menu since it's the only device that's supporting it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data from other devices may differ, yes. So either we use normalized values (= the same data ranges for all devices) or device specific raw values, or even both.

But it would be good to have this for all devices, not just one.

Since some devices provide the data in a different way, where we have no access to all samples at once, we might call a method like samplesAdded(device, start, end) which would then check if CSV export is enabled, then fetch the samples from the given date range from the db (probably cached in the session already) and perform the export.

Alternatively we could integrate with the new DB export functionality and perform the CSV export repeatedly. We would just need to remember the start and end dates so that we only export the incremental changes.


LOG.info("Exporting samples into csv file: " + outFile.getName());
try {
bw = new BufferedWriter(new FileWriter(outFile));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above with try (BufferedWriter ...) and the bw.close()

LOG.info("Exporting samples into csv file: " + outFile.getName());
try {
bw = new BufferedWriter(new FileWriter(outFile));
bw.write("TIMESTAMP" + separator + "DEVICE_ID" + separator + "USER_ID" + separator + "RAW_INTENSITY" + separator + "STEPS" + separator + "RAW_KIND" + separator + "HEART_RATE");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we also export the normalized values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normalized values are deduced from a simple matching rule. I think correlation have to be done after exportation.

public class CSVExport {
private static final Logger LOG = LoggerFactory.getLogger(AbstractSampleProvider.class);

public static void exportToCSV(AbstractActivitySample[] activitySamples, File outFile) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's sufficient to use ActivitySample here instead of AbstractActivitySample.

use try-with-resources statement
Copy link
Contributor

@cpfeiffer cpfeiffer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for being so late, I totally missed your updated PR. I've added two more comments.

// export csv on fetching
File myPath = null;
myPath = FileUtils.getExternalFilesDir();
File myFile = new File(myPath, startTimestamp.getTimeInMillis() / 1000 + ".csv");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data from other devices may differ, yes. So either we use normalized values (= the same data ranges for all devices) or device specific raw values, or even both.

But it would be good to have this for all devices, not just one.

Since some devices provide the data in a different way, where we have no access to all samples at once, we might call a method like samplesAdded(device, start, end) which would then check if CSV export is enabled, then fetch the samples from the given date range from the db (probably cached in the session already) and perform the export.

Alternatively we could integrate with the new DB export functionality and perform the CSV export repeatedly. We would just need to remember the start and end dates so that we only export the incremental changes.

String separator = ",";

LOG.info("Exporting samples into csv file: " + outFile.getName());
try (BufferedWriter bw = new BufferedWriter(new FileWriter(outFile))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileWriter always uses the local platform encoding, which, atm, is UTF-8 on Android. Relying on the local platform encoding will in many cases lead to a bug at some point. One has to use the extra verbose
new OutputStreamWriter(new FileOutputStream(outFile), StandardCharsets.UTF_8))

I've spent too much of my lifetime on encoding bugs already (I actually have a T-Shirt with the text "Schei? Encoding".

For the record, Excel' CSV import does CP-1252 (similar to ISO-8859-1), LibreOffice can handle all kinds of encodings including UTF-8.

@Rahulaswani
Copy link

hI @cpfeiffer @Vebryn was this solution to easily export the activity/sleep data into csv finally implimented, if yes where can i find it ?

I went through the wikis and tried to find a way to export data from the mobile app and found the 'Database Management' option in the side menu which lets you export data but i am not sure of the format, thought it might be sql db but it isn't as DB browser for SQLite cannot recognise and open the file.

Can you help me here or point me in the right direction to be able to easily export my data in a csv file

Thanks

@Vebryn
Copy link
Contributor Author

Vebryn commented Jan 15, 2018

Hi @Rahulaswani,

Export option is available on my branch csv-export. Feel free to contribute.

@Rahulaswani
Copy link

hi @Vebryn i and @izBasit couldn't find the CSV export option in the build from you code. The export options given are exactly the same as the original project. Check screenshot from your repo below

gadgetbridge export option

Can you please help us understand from where can we trigger a csv export or if there is something else that needs to be done ?

@cpfeiffer can you help here ?

@elagin
Copy link
Contributor

elagin commented Aug 27, 2018

I want to try to bring the matter to an end, considering all of the above.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants