Time conversion functions with tests.

Fixes issue with incorrect characters in time strings on UI.
Fix platform dependency


Fix return value with incorrect strings.


Just use strptime and strftime on all platforms.

Emulate strptime on msvc... because they don't have it and their get_time is buggy.

CMakeLists.txt

@@ -37,7 +37,7 @@ set(SLIC3R_GTK "2" CACHE STRING "GTK version to use with wxWidgets on Linux")
 
 # Proposal for C++ unit tests and sandboxes
 option(SLIC3R_BUILD_SANDBOXES   "Build development sandboxes" OFF)
-option(SLIC3R_BUILD_TESTS       "Build unit tests" OFF)
+option(SLIC3R_BUILD_TESTS       "Build unit tests" ON)
 
 # Print out the SLIC3R_* cache options
 get_cmake_property(_cache_vars CACHE_VARIABLES)

src/libslic3r/SLA/SLARasterWriter.cpp

@@ -114,7 +114,7 @@ void SLARasterWriter::set_config(const DynamicPrintConfig &cfg)
     m_config["printerProfile"] = get_cfg_value(cfg, "printer_settings_id");
     m_config["printProfile"]   = get_cfg_value(cfg, "sla_print_settings_id");
 
-    m_config["fileCreationTimestamp"] = Utils::current_utc_time2str();
+    m_config["fileCreationTimestamp"] = Utils::utc_timestamp();
     m_config["prusaSlicerVersion"]    = SLIC3R_BUILD_ID;
 }
 

src/libslic3r/Time.cpp

@@ -3,116 +3,232 @@
 #include <iomanip>
 #include <sstream>
 #include <chrono>
+#include <cassert>
+#include <ctime>
+#include <cstdio>
 
-//#include <boost/date_time/local_time/local_time.hpp>
-//#include <boost/chrono.hpp>
+#ifdef _MSC_VER
+#include <map>
+#endif
 
-
-#ifdef WIN32
-	#define WIN32_LEAN_AND_MEAN
-	#include <windows.h>
-	#undef WIN32_LEAN_AND_MEAN
-#endif /* WIN32 */
+#include "libslic3r/Utils.hpp"
 
 namespace Slic3r {
 namespace Utils {
 
-namespace  {
+// "YYYY-MM-DD at HH:MM::SS [UTC]"
+// If TimeZone::utc is used with the conversion functions, it will append the
+// UTC letters to the end.
+static const constexpr char *const SLICER_UTC_TIME_FMT = "%Y-%m-%d at %T";
 
-// FIXME: after we switch to gcc > 4.9 on the build server, please remove me
-#if defined(__GNUC__) && __GNUC__ <= 4
-std::string put_time(const std::tm *tm, const char *fmt)
+// ISO8601Z representation of time, without time zone info
+static const constexpr char *const ISO8601Z_TIME_FMT = "%Y%m%dT%H%M%SZ";
+
+static const char * get_fmtstr(TimeFormat fmt)
 {
-    static const constexpr int MAX_CHARS = 200;
-    char out[MAX_CHARS];
-    std::strftime(out, MAX_CHARS, fmt, tm);
-    return out;
+    switch (fmt) {
+    case TimeFormat::gcode: return SLICER_UTC_TIME_FMT;
+    case TimeFormat::iso8601Z: return ISO8601Z_TIME_FMT;
+    }
+
+    return "";
 }
-#else
-auto put_time(const std::tm *tm, const char *fmt) -> decltype (std::put_time(tm, fmt))
+
+namespace __get_put_time_emulation {
+// FIXME: Implementations with the cpp11 put_time and get_time either not
+// compile or do not pass the tests on the build server. If we switch to newer
+// compilers, this namespace can be deleted with all its content.
+
+#ifdef _MSC_VER
+// VS2019 implementation fails with ISO8601Z_TIME_FMT.
+// VS2019 does not have std::strptime either. See bug:
+// https://developercommunity.visualstudio.com/content/problem/140618/c-stdget-time-not-parsing-correctly.html
+
+static const std::map<std::string, std::string> sscanf_fmt_map = {
+    {SLICER_UTC_TIME_FMT, "%04d-%02d-%02d at %02d:%02d:%02d"},
+    {std::string(SLICER_UTC_TIME_FMT) + " UTC", "%04d-%02d-%02d at %02d:%02d:%02d UTC"},
+    {ISO8601Z_TIME_FMT, "%04d%02d%02dT%02d%02d%02dZ"}
+};
+
+static const char * strptime(const char *str, const char *const fmt, std::tm *tms)
 {
-    return std::put_time(tm, fmt);
+    auto it = sscanf_fmt_map.find(fmt);
+    if (it == sscanf_fmt_map.end()) return nullptr;
+
+    int y, M, d, h, m, s;
+    if (sscanf(str, it->second.c_str(), &y, &M, &d, &h, &m, &s) != 6)
+        return nullptr;
+
+    tms->tm_year = y - 1900;  // Year since 1900
+    tms->tm_mon  = M - 1;     // 0-11
+    tms->tm_mday = d;         // 1-31
+    tms->tm_hour = h;         // 0-23
+    tms->tm_min  = m;         // 0-59
+    tms->tm_sec  = s;         // 0-61 (0-60 in C++11)
+
+    return str; // WARN strptime return val should point after the parsed string
 }
 #endif
 
+template<class Ttm>
+struct GetPutTimeReturnT {
+    Ttm *tms;
+    const char *fmt;
+    GetPutTimeReturnT(Ttm *_tms, const char *_fmt): tms(_tms), fmt(_fmt) {}
+};
+
+using GetTimeReturnT = GetPutTimeReturnT<std::tm>;
+using PutTimeReturnT = GetPutTimeReturnT<const std::tm>;
+
+std::ostream &operator<<(std::ostream &stream, PutTimeReturnT &&pt)
+{
+    static const constexpr int MAX_CHARS = 200;
+    char _out[MAX_CHARS];
+    strftime(_out, MAX_CHARS, pt.fmt, pt.tms);
+    stream << _out;
+    return stream;
 }
 
-time_t parse_time_ISO8601Z(const std::string &sdate)
+inline PutTimeReturnT put_time(const std::tm *tms, const char *fmt)
 {
-	int y, M, d, h, m, s;
-	if (sscanf(sdate.c_str(), "%04d%02d%02dT%02d%02d%02dZ", &y, &M, &d, &h, &m, &s) != 6)
-        return time_t(-1);
-	struct tm tms;
-    tms.tm_year = y - 1900;  // Year since 1900
-	tms.tm_mon  = M - 1;     // 0-11
-	tms.tm_mday = d;         // 1-31
-	tms.tm_hour = h;         // 0-23
-	tms.tm_min  = m;         // 0-59
-	tms.tm_sec  = s;         // 0-61 (0-60 in C++11)
+    return {tms, fmt};
+}
+
+std::istream &operator>>(std::istream &stream, GetTimeReturnT &&gt)
+{
+    std::string line;
+    std::getline(stream, line);
+
+    if (strptime(line.c_str(), gt.fmt, gt.tms) == nullptr)
+        stream.setstate(std::ios::failbit);
+
+    return stream;
+}
+
+inline GetTimeReturnT get_time(std::tm *tms, const char *fmt)
+{
+    return {tms, fmt};
+}
+
+}
+
+namespace {
+
+// Platform independent versions of gmtime and localtime. Completely thread
+// safe only on Linux. MSVC gtime_s and localtime_s sets global errno thus not
+// thread safe.
+struct std::tm * _gmtime_r(const time_t *timep, struct tm *result)
+{
+    assert(timep != nullptr && result != nullptr);
 #ifdef WIN32
-	return _mkgmtime(&tms);
+    time_t t = *timep;
+    gmtime_s(result, &t);
+    return result;
+#else
+    return gmtime_r(timep, result);
+#endif
+}
+
+struct std::tm * _localtime_r(const time_t *timep, struct tm *result)
+{
+    assert(timep != nullptr && result != nullptr);
+#ifdef WIN32
+    // Converts a time_t time value to a tm structure, and corrects for the
+    // local time zone.
+    time_t t = *timep;
+    localtime_s(result, &t);
+    return result;
+#else
+    return localtime_r(timep, result);
+#endif
+}
+
+time_t _mktime(const struct std::tm *tms)
+{
+    assert(tms != nullptr);
+    std::tm _tms = *tms;
+    return mktime(&_tms);
+}
+
+time_t _timegm(const struct std::tm *tms)
+{
+    std::tm _tms = *tms;
+#ifdef WIN32
+    return _mkgmtime(&_tms);
 #else /* WIN32 */
-	return timegm(&tms);
+    return timegm(&_tms);
 #endif /* WIN32 */
 }
 
-std::string format_time_ISO8601Z(time_t time)
+std::string process_format(const char *fmt, TimeZone zone)
 {
-	struct tm tms;
-#ifdef WIN32
-	gmtime_s(&tms, &time);
-#else
-	gmtime_r(&time, &tms);
-#endif
-	char buf[128];
-	sprintf(buf, "%04d%02d%02dT%02d%02d%02dZ",
-    	tms.tm_year + 1900,
-		tms.tm_mon + 1,
-		tms.tm_mday,
-		tms.tm_hour,
-		tms.tm_min,
-		tms.tm_sec);
-	return buf;
+    std::string fmtstr(fmt);
+
+    if (fmtstr == SLICER_UTC_TIME_FMT && zone == TimeZone::utc)
+        fmtstr += " UTC";
+
+    return fmtstr;
 }
 
-std::string format_local_date_time(time_t time)
-{
-	struct tm tms;
-#ifdef WIN32
-	// Converts a time_t time value to a tm structure, and corrects for the local time zone.
-	localtime_s(&tms, &time);
-#else
-	localtime_r(&time, &tms);
-#endif
-    char buf[80];
- 	strftime(buf, 80, "%x %X", &tms);
-    return buf;
-}
+} // namespace
 
 time_t get_current_time_utc()
-{    
+{
     using clk = std::chrono::system_clock;
     return clk::to_time_t(clk::now());
 }
 
-static std::string tm2str(const std::tm *tm, const char *fmt)
+static std::string tm2str(const std::tm *tms, const char *fmt)
 {
     std::stringstream ss;
-    ss << put_time(tm, fmt);
+    ss.imbue(std::locale("C"));
+    ss << __get_put_time_emulation::put_time(tms, fmt);
     return ss.str();
 }
 
-std::string time2str(const time_t &t, TimeZone zone, const char *fmt)
+std::string time2str(const time_t &t, TimeZone zone, TimeFormat fmt)
 {
     std::string ret;
-    
+    std::tm tms = {};
+    tms.tm_isdst = -1;
+    std::string fmtstr = process_format(get_fmtstr(fmt), zone);
+
     switch (zone) {
-    case TimeZone::local: ret = tm2str(std::localtime(&t), fmt); break;
-    case TimeZone::utc:   ret = tm2str(std::gmtime(&t), fmt) + " UTC"; break;
+    case TimeZone::local:
+        ret = tm2str(_localtime_r(&t, &tms), fmtstr.c_str()); break;
+    case TimeZone::utc:
+        ret = tm2str(_gmtime_r(&t, &tms), fmtstr.c_str()); break;
     }
-    
+
     return ret;
 }
 
+static time_t str2time(std::istream &stream, TimeZone zone, const char *fmt)
+{
+    std::tm tms = {};
+    tms.tm_isdst = -1;
+
+    stream >> __get_put_time_emulation::get_time(&tms, fmt);
+    time_t ret = time_t(-1);
+
+    switch (zone) {
+    case TimeZone::local: ret = _mktime(&tms); break;
+    case TimeZone::utc:   ret = _timegm(&tms); break;
+    }
+
+    if (stream.fail() || ret < time_t(0)) ret = time_t(-1);
+
+    return ret;
+}
+
+time_t str2time(const std::string &str, TimeZone zone, TimeFormat fmt)
+{
+    std::string fmtstr = process_format(get_fmtstr(fmt), zone).c_str();
+    std::stringstream ss(str);
+
+    ss.imbue(std::locale("C"));
+    return str2time(ss, zone, fmtstr.c_str());
+}
+
 }; // namespace Utils
 }; // namespace Slic3r

src/libslic3r/Time.hpp

@@ -7,41 +7,61 @@
 namespace Slic3r {
 namespace Utils {
 
-// Utilities to convert an UTC time_t to/from an ISO8601 time format,
-// useful for putting timestamps into file and directory names.
-// Returns (time_t)-1 on error.
-time_t parse_time_ISO8601Z(const std::string &s);
-std::string format_time_ISO8601Z(time_t time);
-
-// Format the date and time from an UTC time according to the active locales and a local time zone.
-// TODO: make sure time2str is a suitable replacement
-std::string format_local_date_time(time_t time);
-
-// There is no gmtime() on windows.
+// Should be thread safe.
 time_t get_current_time_utc();
 
-const constexpr char *const SLIC3R_TIME_FMT = "%Y-%m-%d at %T";
-
 enum class TimeZone { local, utc };
+enum class TimeFormat { gcode, iso8601Z };
 
-std::string time2str(const time_t &t, TimeZone zone, const char *fmt = SLIC3R_TIME_FMT);
+// time_t to string functions...
 
-inline std::string current_time2str(TimeZone zone, const char *fmt = SLIC3R_TIME_FMT)
+std::string time2str(const time_t &t, TimeZone zone, TimeFormat fmt);
+
+inline std::string time2str(TimeZone zone, TimeFormat fmt)
 {
     return time2str(get_current_time_utc(), zone, fmt);
 }
 
-inline std::string current_local_time2str(const char * fmt = SLIC3R_TIME_FMT)
+inline std::string utc_timestamp(time_t t)
 {
-    return current_time2str(TimeZone::local, fmt);    
+    return time2str(t, TimeZone::utc, TimeFormat::gcode);
 }
 
-inline std::string current_utc_time2str(const char * fmt = SLIC3R_TIME_FMT)
+inline std::string utc_timestamp()
 {
-    return current_time2str(TimeZone::utc, fmt);
+    return utc_timestamp(get_current_time_utc());
 }
 
-}; // namespace Utils
-}; // namespace Slic3r
+// String to time_t function. Returns time_t(-1) if fails to parse the input.
+time_t str2time(const std::string &str, TimeZone zone, TimeFormat fmt);
+
+
+// /////////////////////////////////////////////////////////////////////////////
+// Utilities to convert an UTC time_t to/from an ISO8601 time format,
+// useful for putting timestamps into file and directory names.
+// Returns (time_t)-1 on error.
+
+// Use these functions to convert safely to and from the ISO8601 format on
+// all platforms
+
+inline std::string iso_utc_timestamp(time_t t)
+{
+    return time2str(t, TimeZone::utc, TimeFormat::gcode);
+}
+
+inline std::string iso_utc_timestamp()
+{
+    return iso_utc_timestamp(get_current_time_utc());
+}
+
+inline time_t parse_iso_utc_timestamp(const std::string &str)
+{
+    return str2time(str, TimeZone::utc, TimeFormat::iso8601Z);
+}
+
+// /////////////////////////////////////////////////////////////////////////////
+
+} // namespace Utils
+} // namespace Slic3r
 
 #endif /* slic3r_Utils_Time_hpp_ */

src/libslic3r/utils.cpp

@@ -543,7 +543,7 @@ std::string string_printf(const char *format, ...)
 
 std::string header_slic3r_generated()
 {
-    return std::string("generated by " SLIC3R_APP_NAME " " SLIC3R_VERSION " on " ) + Utils::current_utc_time2str();
+    return std::string("generated by " SLIC3R_APP_NAME " " SLIC3R_VERSION " on " ) + Utils::utc_timestamp();
 }
 
 unsigned get_current_pid()

src/slic3r/Config/Snapshot.cpp

@@ -66,7 +66,7 @@ void Snapshot::load_ini(const std::string &path)
                 if (kvp.first == "id")
                     this->id = kvp.second.data();
                 else if (kvp.first == "time_captured") {
-                	this->time_captured = Slic3r::Utils::parse_time_ISO8601Z(kvp.second.data());
+                	this->time_captured = Slic3r::Utils::parse_iso_utc_timestamp(kvp.second.data());
 					if (this->time_captured == (time_t)-1)
 				        throw_on_parse_error("invalid timestamp");
                 } else if (kvp.first == "slic3r_version_captured") {
@@ -165,7 +165,7 @@ void Snapshot::save_ini(const std::string &path)
     // Export the common "snapshot".
 	c << std::endl << "[snapshot]" << std::endl;
 	c << "id = " << this->id << std::endl;
-	c << "time_captured = " << Slic3r::Utils::format_time_ISO8601Z(this->time_captured) << std::endl;
+	c << "time_captured = " << Slic3r::Utils::iso_utc_timestamp(this->time_captured) << std::endl;
 	c << "slic3r_version_captured = " << this->slic3r_version_captured.to_string() << std::endl;
 	c << "comment = " << this->comment << std::endl;
 	c << "reason = " << reason_string(this->reason) << std::endl;
@@ -365,7 +365,7 @@ const Snapshot&	SnapshotDB::take_snapshot(const AppConfig &app_config, Snapshot:
 	Snapshot snapshot;
 	// Snapshot header.
 	snapshot.time_captured 			 = Slic3r::Utils::get_current_time_utc();
-	snapshot.id 					 = Slic3r::Utils::format_time_ISO8601Z(snapshot.time_captured);
+	snapshot.id 					 = Slic3r::Utils::iso_utc_timestamp(snapshot.time_captured);
 	snapshot.slic3r_version_captured = Slic3r::SEMVER;
 	snapshot.comment 				 = comment;
 	snapshot.reason 				 = reason;

src/slic3r/GUI/ConfigSnapshotDialog.cpp

@@ -35,9 +35,14 @@ static wxString generate_html_row(const Config::Snapshot &snapshot, bool row_eve
     text += snapshot_active ? "#B3FFCB" : (row_even ? "#FFFFFF" : "#D5D5D5");
     text += "\">";
     text += "<td>";
+    
+    static const constexpr char *LOCALE_TIME_FMT = "%x %X";
+    wxString datetime = wxDateTime(snapshot.time_captured).Format(LOCALE_TIME_FMT);
+    
     // Format the row header.
-    text += wxString("<font size=\"5\"><b>") + (snapshot_active ? _(L("Active")) + ": " : "") + 
-        Utils::format_local_date_time(snapshot.time_captured) + ": " + format_reason(snapshot.reason);
+    text += wxString("<font size=\"5\"><b>") + (snapshot_active ? _(L("Active")) + ": " : "") +
+            datetime + ": " + format_reason(snapshot.reason);
+    
     if (! snapshot.comment.empty())
         text += " (" + wxString::FromUTF8(snapshot.comment.data()) + ")";
     text += "</b></font><br>";

tests/CMakeLists.txt

@@ -1,3 +1,13 @@
 # TODO Add individual tests as executables in separate directories
+# add_subirectory(<testcase>)
 
-# add_subirectory(<testcase>)
+find_package(GTest REQUIRED)
+
+set(TEST_DATA_DIR ${CMAKE_CURRENT_SOURCE_DIR}/data)
+file(TO_NATIVE_PATH "${TEST_DATA_DIR}" TEST_DATA_DIR)
+
+add_library(test_common INTERFACE)
+target_compile_definitions(test_common INTERFACE TEST_DATA_DIR="${TEST_DATA_DIR}")
+target_link_libraries(test_common INTERFACE GTest::GTest GTest::Main)
+
+add_subdirectory(timeutils)

tests/timeutils/CMakeLists.txt

@@ -0,0 +1,5 @@
+add_executable(timeutils_tests timeutils_tests_main.cpp)
+target_link_libraries(timeutils_tests test_common libslic3r ${Boost_LIBRARIES} ${TBB_LIBRARIES} ${Boost_LIBRARIES})
+
+add_test(timeutils_tests timeutils_tests)
+#gtest_discover_tests(timeutils_tests TEST_PREFIX timeutils.)

tests/timeutils/timeutils_tests_main.cpp

@@ -0,0 +1,41 @@
+#include <gtest/gtest.h>
+#include "libslic3r/Time.hpp"
+
+#include <sstream>
+#include <iomanip>
+#include <locale>
+
+namespace {
+
+void test_time_fmt(Slic3r::Utils::TimeFormat fmt) {
+    using namespace Slic3r::Utils;
+    time_t t = get_current_time_utc();
+    
+    std::string tstr = time2str(t, TimeZone::local, fmt);
+    time_t parsedtime = str2time(tstr, TimeZone::local, fmt);
+    ASSERT_EQ(t, parsedtime);
+    
+    tstr = time2str(t, TimeZone::utc, fmt);
+    parsedtime = str2time(tstr, TimeZone::utc, fmt);
+    ASSERT_EQ(t, parsedtime);
+    
+    parsedtime = str2time("not valid string", TimeZone::local, fmt);
+    ASSERT_EQ(parsedtime, time_t(-1));
+    
+    parsedtime = str2time("not valid string", TimeZone::utc, fmt);
+    ASSERT_EQ(parsedtime, time_t(-1));
+}
+}
+
+TEST(Timeutils, ISO8601Z) {
+    test_time_fmt(Slic3r::Utils::TimeFormat::iso8601Z);
+}
+
+TEST(Timeutils, Slic3r_UTC_Time_Format) {
+    test_time_fmt(Slic3r::Utils::TimeFormat::gcode);
+}
+
+int main(int argc, char **argv) {
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}