Closed luisdavidgarcia closed 1 month ago
I like the direction this is headed.
It brings up a good question: when do you attach the timestamp? On the tx or rx side? This uses rx side, which is valid. My one concern would be if receiving stale data you wouldn't actually know when it was sampled. In other words, the assumption is that the time at which the data is received is approximately equal to the time at which it was sampled. An example of when this might not be the case is when the Arduino is connected to over serial -- it finishes its last tx, resets, and then continues to tx. On the rx side it can be difficult to differentiate the data from before and after the reset since the rx is buffered.
Sorry about the comments, the place I worked at was quite rigorous in their code reviews.
In my ideal world, I would have two tasks setup in the MCU:
In this case testing would need to be done to see if an overflow occurs in the tx queue. If so, adjustments to sample frequency would have to be made. (Like queuing data in every other interrupt, etc.)
However it might be simpler to modify the code to include the timestamp in the collection of print statements it currently has. See https://github.com/kennedyengineering/EE525_Final_Project/blob/a8ea29e6a6ddec4aeefceb3ba539bf6ca5566b69/src/Arduino/main/main.ino#L37
From my experience in dealing with post processing data, accurate timing information is everything.
Made what I think is the easiest change. See MR #2
Since screen can work as a Linux/OSX solution, maybe we don't reinvent the wheel with this logger.py program? What if we created a src/Scripts directory and created a bash script that did the logging using screen? We could save the src/Python directory for post-processing scripts maybe...
Synthesized ideas into an issue #3
Updated the logger script with two CLI arguments one for log_dir and csv_file_name (name of the log file).
Also removed the print statements of the data since it is now being outputted to a CSV file.