Refactoring the LoggingManager - V1.0

[Vincent-SD]

See med-material/LoggingManager#13

• Adding the logStore class
  • Ability to create the log string on the fly or at the end of the logging process by setting a boolean to true or false in Unity.
• Adaptation of the LoggingManager
• Adding the WriteToCSV class

The MySQL part is not yet implemented and adapted in this version.

[bastianilso]

OK, thanks for showing me the state of things Vincent 🥥.

I think we should structure the pull requests the following way:

• you continue working on the implementation in ArduinoLogger, if this is what is most comfortable for you (and a nice testing bed).
• We then test that the LoggingManager works in ArduinoLogger, Whack-A-Mole VR and HandStrengthener.
• After this is confirmed, I'd like you to submit a pull request to the LoggingManager repo itself.
• Ideally, we structure your changes the following way: Commit 1) Change Dictionary to list structure, Commit 2) Create LogStore class, 3) Convert CSV Writing to WriteToCSV class, 4) Convert SQL class, 5) style fixes and misc. Note, the point is not necessarily that you make the code work between each commit, but instead that it becomes easier for me to review your code. Dont worry about the structure right now - feel free to postpone this to later.
• After upstream LoggingManager is updated, we make a new release and Unity package.
• The applications using LoggingManager can then update by importing this Unity package and override the existing implementation.

[Vincent-SD]

I agree, I am currently working on the loggingManager to make it work in Whack-A-Mole VR with Quentin. After that i will see if it is working too with HandStrengthener and I will restructure the commits.

[Vincent-SD]

Structure of the LoggingManager

The LoggingManager now has 2 new classes : LogStore and WriteToCSV.

Logstore

This class will be used to store the logs and organize them in the correct format. Then, it will be possible to export the logs in a string format or in a dictionary format (see med-material/LoggingManager#13).

WriteToCSV

This class will allow to save the logs in a file.

Implementation of LogStore class

This is the implementation of all the functions in the LogStore class. Here are useful things to know about this class.

• The first row logged will define the headers of the logs

• It is possible to enable/disable the functionality that creates the string on the fly (during the log process)

  • If an header is added during the log process :
    • A new header is created and the "NULL" value is put for all the previous rows
    • If the log string on the fly functionality is enabled, it will be disabled

• Each time a line is fully logged, the TerminateRow() function has to be called. In order for this new version to work with previous implementations of the LoggingManager, this function is called automatically when needed (as explained here).

Implementation of LogToCSV class

Each LogToCSV object is instantiated with a file path and a LogStore object. When calling the WriteAll() function, the logs are exported from the LogStore object as a string and are written in the file at the given file path. The process is currently executed in a separated thread to prevent the main application from freezing.

[Vincent-SD]

@bastianilso I can't request review for some unknown reason, so I'm commenting on this pull request🌴

[bastianilso]

@Vincent-SD great! One last request: could you split the your changes to LoggingManager.cs and LoggingExample.cs into a separate commit that happens before the changes to ArduinoLogger? ❄

[Vincent-SD]

I updated the Pull Request. Regarding the enum, i put comments but I didn't change the labels :

public enum LogType
{
    Normal, //Regular logs - logs automatically commons columns like TimeStamp or Framecount
    Meta //Meta logs - logs only one line containing at least sessionID and Email
}

I did this because the only difference between the 2 LogType is the common columns that are logged each row (with the Meta type, it is still possible to log more than one row, should I make this impossible ?). In this case, the types correspond well to their real behavior : one type for the Regular logs, one type for the Meta logs.

[bastianilso]

@Vincent-SD Thanks, I will have a look at your update and resolve the conversations above as I see fit.

with the Meta type, it is still possible to log more than one row, should I make this impossible?

1. yes, the Meta type should only be able to log one row. That is the whole definition of Meta - that if there is ever written data to a logtype with this type, the existing data is overwritten. Because, the type of data inside a Meta logtype, describes the logging itself - columns cannot take more than 1 value and if they do, things will break in data analysis.

I did this because the only difference between the 2 LogType is the common columns that are logged each row In this case, the types correspond well to their real behavior : one type for the Regular logs, one type for the Meta logs.

"Logged each row" - I think this is much more descriptive of what this enum actually does than "Regular" or "Normal".

So why not call the Normal option e.g. LogType.RowForRow or LogType.LogEachRow ? Something that describes what choosing the type does. I think it tells developers more about the behavior than LogType.Normal or LogType.Regular.

in a similar way, Meta could be called LogType.OneRowOverwrite or something like that.

[Vincent-SD]

I moved all the changes concerning the old arduinoLogger to the "Adding the old LoggingManager" commit.

I also changed the behavior of the meta logs (OneRowOverwrite) so that only one line can be logged. These (small) changes are in the "Implementation of LogStore class" commit.

I fixed some bugs I found too 🔧.

[hendrikknoche]

I think it would make sense to include this comment/justification why the code has the structure as part of the code documentation. E.g. to avoid future ‘corrections’. Can you include some explanations, Vincent? Best, -Hendrik

From: Bastian Ilso @.> Reply to: med-material/ArduinoLogger @.> Date: Monday, 3 January 2022 at 20:14 To: med-material/ArduinoLogger @.> Cc: Subscribed @.> Subject: Re: [med-material/ArduinoLogger] Refactoring the LoggingManager - V1.0 (PR #46)

@bastianilso commented on this pull request.

---

In Assets/LoggingManager/LoggingManager.cshttps://github.com/med-material/ArduinoLogger/pull/46#discussion_r777657500:

• public void TerminateLogRow(string collectionLabel)

• {

• logsList[collectionLabel].EndRow();

• }

@Vincent-SDhttps://github.com/Vincent-SD The main issue with this logic, is that it makes reading Timestamp, Framecount etc. for each row problematic - there is no longer a forced correlation between the reported timestamp and the time which the values were created (it could become arbitrary). It means, that if users are not aware of this, they can run into a lot of trouble in data analysis.

This is the main reason that the LoggingManager was designed to always terminate the line, after Log() is called - and to force a Timestamp on every single row. This behavior was not a limitation/simplification, but a feature.

Unfortunately it is very important to maintain this correlation for any type of temporal data analysis, so I don't think we should put a boolean in Log() for this.

— Reply to this email directly, view it on GitHubhttps://github.com/med-material/ArduinoLogger/pull/46#discussion_r777657500, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAPEDH6WSDLKVINUDWYH74TUUHYRJANCNFSM5ITDQTZA. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you are subscribed to this thread.Message ID: @.***>