Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Time Zone and Daylight Saving Time Customization for MDF Data #103

Merged
merged 11 commits into from
Sep 11, 2024

Conversation

simonkimi
Copy link
Collaborator

I needed to customize the time zone and daylight saving time for MDF data, but these features are currently not available. Additionally, the time handling between MDF3 and MDF4 is different—MDF4 defaults to UTC time, while MDF3 defaults to local time. Considering that some users might already be using these functions, I have added three new functions: SetStartTimeLocal, SetStartTimeUtc, and SetStartTimeWithZone.

Please note that since I do not have access to the official MDF file format documentation (it appears to be a paid resource), the data was inferred using the Vector MDF Validator. There might be some issues as a result.

# Conflicts:
#	mdflibrary/src/MdfHeader.cpp
#	mdflibrary/src/MdfHeader.h
@ihedvall ihedvall added the enhancement New feature or request label Aug 28, 2024
@ihedvall
Copy link
Owner

Just for your information. I did removed all DST/TZ settings from the interface by design because it is bad idea to involve DST/TZ information in the MDF file. It's OK to have functions for DST/TZ settings but I recommend not to use them. It normally just ends up as a problem later on.

There should be some issue in the MDFWriter::StartMeasurement(startTime) function. This function modifies the header start time. The reason is somewhat difficult to describe but the pre-trig time and the internal sample cache are involved. Inside, I use UTC for timestamps , so we need to find some condition when to set and not to set the header start time. A samples absolute time is header start time + sample relative time. Note that the relative time can be negative.

@simonkimi
Copy link
Collaborator Author

Due to a lack of documentation, I would like to ask some questions: The definition of StartTime is Absolute start time in ns since midnight Jan 1st, 1970, does it include time zone information? If it's a timestamp, it should be without time zone

@ihedvall
Copy link
Owner

MDF 4 definition of Start Time (same for File Header)

Note there is currently no Time Flags defined. Proposal from me is that the Mdf4TimeStamp.h/cpp class exist but it is not public. Make an ITimeStamp class that defines the property interface and use this interface for getting/setting non-UTC absolute times. Keep the UTC time functions as is.

  • Start Time. Time stamp at start of measurement in nanoseconds elapsed since 00:00:00 01.01.1970 (UTC time or local time, depending on "local time" flag, see [20]). All time stamps for time synchronized master channels or events are always relative to this start time stamp.
  • TZ Offset. Time zone offset in minutes. The value is not necessarily a multiple of 60 and can be negative! For the current time zone definitions, it is expected to be in the range [-840,840] min. For example a value of 60 (min) means UTC+1 time zone = Central European Time (CET).
  • DST Offset. Daylight saving time (DST) offset in minutes for start time stamp. During the summer months, most regions observe a DST offset of 60 min (1 hour). Only valid if "time offsets valid" flag is set in time flags.
  • Time Flags. Bit 0: Local time flag
    If set, the start time stamp in nanoseconds represents the local time instead of the UTC time. In this case, time zone and DST offset must not be considered (time offsets flag must not be set). Should only be used if UTC time is unknown.
    If the bit is not set (default), the start time stamp represents the UTC time. Bit 1: Time offsets valid flag
    If set, the time zone and DST offsets are valid. Must not be set together with "local time" flag (mutually exclusive).
    If the offsets are valid, the locally displayed time at start of recording can be determined (after conversion of offsets to ns) by
    Local time = UTC time + time zone offset + DST offset.

@simonkimi
Copy link
Collaborator Author

Alright, I will create an ITimestamp to set non-UTC times, but I have a question. When the flag is 0 or 2, the stored timestamp is in UTC and there are no issues. However, when the flag is 1, the stored local time must be adjusted by adding the timezone and daylight saving time offsets to display the current time in the Vector MDF Validator. Is this adjustment process done automatically by the library or does it need to be manually added by the user?

If we allow the library to automatically make the adjustment, there might be issues with files generated over a long period experiencing time rollbacks due to daylight saving time changes. I would prefer that the user manually handle this to have more flexibility.

@ihedvall
Copy link
Owner

It's up to you as the ITimestamp interface is new. If you have 4 properties (flag, time, dst_offset and tz_offset), The time (ns_1970) is dependent on the bit 0 flag. You can keep the current UTC functions but rename them NsSince1970->UtcSince1970 (or similar).

The bit 1 flag defines if the offsets a valid or not. I suppose that these should be initialized in the constructor. In C++ it's a mess to getting these values however (Windows vs Linux).

The .NET have a DateTime built into System, so the ITimestamp interface in MdfLibrary should use that DateTime structure.

The MDF 3 always uses local time without DST offset only TZ offset.

As before, try to use UTC only in your applications. It solves a lot of issues. Convert to local time format when presenting in an user interface.

@simonkimi simonkimi changed the title Add Time Zone and Daylight Saving Time Customization for MDF Data Drift: Add Time Zone and Daylight Saving Time Customization for MDF Data Aug 29, 2024
@simonkimi simonkimi marked this pull request as draft August 29, 2024 09:06
@simonkimi simonkimi changed the title Drift: Add Time Zone and Daylight Saving Time Customization for MDF Data Add Time Zone and Daylight Saving Time Customization for MDF Data Aug 29, 2024
@ihedvall
Copy link
Owner

I suspect that you forget to add the new timestamp.h/cpp files to the cmake project file (mdflib/CMakeLists.txt). Just a minor fix.

The MDF 3 specification is open source. The HD block includes both text and numeric times. It's a mess with DST and non-DST but always local time. Note that MDF 3 writing is seldom used.

image

@simonkimi
Copy link
Collaborator Author

I should convert it into a draft before submitting it. The time part of the mdf3 section is not completed due to some uncertainties. I have already obtained the mdf3 document, and next, I will refer to the document for development.

@ihedvall
Copy link
Owner

Just a hint. There are 2 examples at the end of the page. This could be the input for a unit test that test the HD3 times.

@simonkimi
Copy link
Collaborator Author

The development of the feature has been completed, and the unit tests have passed on my local machine. However, since I'm not very familiar with C++/CLI, I've tried various methods but still can't resolve the compilation errors in the CI unit tests. I might need some help with this.

@ihedvall
Copy link
Owner

ihedvall commented Sep 9, 2024

I made a fast analyze of the GitHub Action logs and it is the MdfLibrary (.NET assembly) linking by CMAKE that complains. The linker error messages are not the most user friendly but it is missing some code. I suspect that the MdfLibrary::CMakeLists.txt is missing some new files. Can't find the MdfUtfTimestamp and MdfLocalTimestamp source files in GIT. It could be some files missing in the C++ MdfLib as well.

MdfWriter.obj : error LNK2020: unresolved token (060000DE) MdfLibrary.MdfUtcTimestamp::.ctor [D:\a\mdflib\mdflib\build\msvc-vcpkg-static-x64\mdflibrary\mdflibrary.vcxproj]

The .NET CMAKE build is only made by one GitHub Action. I have not figure out how to build a Visual Studio Solution file with GitHub Action but I save that for a rainy day.

@simonkimi
Copy link
Collaborator Author

That makes sense then. My Visual Studio probably didn't use CMake but directly used the built-in vcxproj for building. I'll give it another try.

@simonkimi simonkimi marked this pull request as ready for review September 10, 2024 02:23
@ihedvall ihedvall merged commit 482bb14 into ihedvall:main Sep 11, 2024
5 checks passed
@ihedvall
Copy link
Owner

@simonkimi
Thanks for a great work.
Ingemar Hedvall

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

2 participants