Closed Sarci1 closed 4 years ago
Bleep bloop!
LabVIEW Diff Robot here with some diffs served up hot for your pull request.
Notice something funny? Help fix me on my GitHub repo.
The following VIs could not be diffed:
Bleep bloop!
LabVIEW Diff Robot here with some diffs served up hot for your pull request.
Notice something funny? Help fix me on my GitHub repo.
The following VIs could not be diffed:
- [X] Delete
Convert Span Size to Time.vi
. It is not used anywhere. Remove the corresponding diagram disable structure and related code for getting the engine rate inInitialize Logging Data.vi
.- [X] Revert
Sample File Data.vi
andSet Data in Group Buffer.vi
. There are no actual changes to these VIs.- [X]
Asynchronous Logging Loop.vi
andUpdate Log Start Time in New Trigger.vi
need to have source separated.- [X]
test Log Span Size.vi
fails. The TDMS file has more in it than just the raw data that is written, so the file size will not be exactly 1000000 bytes. Revert the test VI to test the original file size that was there. This value accounts for the size of the metadata.- [X]
test Log Span Size.vi
has a 6 minute wait. With the performance improvements, it only takes about 30 seconds for the file to span. Instead of waiting for some period of time, put in a check for the number of TDMS files in the directory and when there are multiple files, check the size of the oldest. Also make sure to have something like a 1 minute timeout in case spanning actually does fail. This will greatly speed up this test.Not a huge deal, but just wanted to verify: Using the millisecond timer limits us to about 50 days of logging for a single file (whereas the timestamp has no limitation). This is probably ok, but I just wanted to make sure we don't have a use case that might require longer time before spanning. Is this limitation acceptable?
All the requests should have been addressed in my latest commits. As for the limitation. I have no problem with this. I don't expect anyone to want to log to a single file for 50 days. For our use case, 1 day max would be expected.
Bleep bloop!
LabVIEW Diff Robot here with some diffs served up hot for your pull request.
Notice something funny? Help fix me on my GitHub repo.
The following VIs could not be diffed:
- [X] @niphilj would like to have the time span implementation using the Timestamp instead of millisecond timer so we don't have any artificial restrictions on duration. I like the changes you have for encapsulating the time span, so this is just replacing the U32 in your implementation with the Timestamp.
- [X] In
Embedded Data Logger Windows System Tests.lvclass:Wait for Files.vi
, remove the last case structure and just use the JKIfailIf.vi
. This gives you the same behavior, but makes the diagram more readable.After these changes, this should be ready to go.
This should be addressed in the latest commit.
Bleep bloop!
LabVIEW Diff Robot here with some diffs served up hot for your pull request.
Notice something funny? Help fix me on my GitHub repo.
The following VIs could not be diffed:
- [X]
test Log Span Time.vi
is failing because it is logging for 30 seconds instead of 20.
I had forgotten to remove the conversion from ms to s.
TODO: Check the above box with an 'x' indicating you've read and followed CONTRIBUTING.md.
What does this Pull Request accomplish?
Why should this Pull Request be merged?
20 is addressed
What testing has been done?
Validated locally on Windows PC for performance across multiple file sizes with varying number of groups. Ensured Missed Packets did not grow.