Add TDMS Waveform Properties to Channels

[oscarfonloz]

• [x] This contribution adheres to CONTRIBUTING.md.

TODO: Check the above box with an 'x' indicating you've read and followed CONTRIBUTING.md.

What does this Pull Request accomplish?

Adds the following properties to the TDMS channels:

• wf_xname
• wf_xunit_string
• wf_samples
• wf_time_pref
• wf_start_time

Why should this Pull Request be merged?

It will allow customers to see the log files with the correct time axis in SystemLink, DIAdem, and the LabVIEW TDMS Viewer.

What testing has been done?

TODO: Detail what testing has been done to ensure this submission meets requirements.

[niveristand-diff-bot]

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.

Embedded Data Logger Engine.lvlib--Open Data Log File.vi.png


Embedded Data Logger Engine.lvlib--Set Default Channel Properties.vi.png


[rtzoeller]

I've not functionally tested the changes yet, but from a quick code review:

• [ ] We're reading the start time in multiple places; once in Open Data Log File.vi and once in Set Default Channel Properties.vi (actually n times, as it's in a loop). This is a potential data integrity issue. The existing code attempts to read the start time as late as possible (I believe for accuracy, i.e. to not introduce a delay between the reported start time and the actual start time); we probably should continue doing that. I'd prefer to see us write all properties except wf_start_time first, for all channels, then write just the wf_start_time property for all channels right before returning from Open Data Log File.vi. All channels should use the same time.
• [ ] No need to mention NI or R&D in the comment. It can be more concisely stated as "1 sample indicates the acquisition is continuous and the size is not known in advance."

[oscarfonloz]

I've not functionally tested the changes yet, but from a quick code review:

• [ ] We're reading the start time in multiple places; once in Open Data Log File.vi and once in Set Default Channel Properties.vi (actually n times, as it's in a loop). This is a potential data integrity issue. The existing code attempts to read the start time as late as possible (I believe for accuracy, i.e. to not introduce a delay between the reported start time and the actual start time); we probably should continue doing that. I'd prefer to see us write all properties except wf_start_time first, for all channels, then write just the wf_start_time property for all channels right before returning from Open Data Log File.vi. All channels should use the same time.

I agree, and I'm currently speaking with @debryant about the implications of this start time for situations when we don't start logging immediately after creating a file, as well as when we have pre-trigger samples... I might ping you later to get your thoughts about it. Mainly to understand how these situations are currently handled, so we can add the functionality without spending too much time figuring out where it fits.

• [x] No need to mention NI or R&D in the comment. It can be more concisely stated as "1 sample indicates the acquisition is continuous and the size is not known in advance."

[niveristand-diff-bot]

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.

Embedded Data Logger Engine.lvlib--Open Data Log File.vi.png


Embedded Data Logger Engine.lvlib--Set Default Channel Properties.vi.png


[dbendele]

this may be a useful reference (or it may need updating depending on PR outcomes): https://nio365.sharepoint.com/sites/tdms-fileformat/SitePages/Application%20specific%20extensions%20to%20the%20TDM%20model.aspx

[dbendele]

• [ ] In Embedded Data Logger Engine.lvlib--Open Data Log File.vi, write the File.Created property once, after writing the channel properties. This would allow file Created property and File Start Time to use same timestamp.

[dbendele]

• [ ] In Embedded Data Logger Engine.lvlib--Open Data Log File.vi recommend reshaping the channel property data so that each channel's properties can be written in one call to TDMS Set Properties rather than multiple calls.

[oscarfonloz]

For the PR's visibility: I had a meeting with Ryan, Donovan, and Deborah today and we decided on how to move forward with the feature. I will be making the appropriate changes through the week.

[oscarfonloz]

From @rtzoeller: "@Oscar Fonseca can you close the existing PR so we get a blank slate, and open a new one when it's ready?" Hence, closing it. @dbendele thank you so much for the suggestions, I will definitely use them!!