openbmc / sdbusplus

C++ bindings for systemd dbus APIs
Apache License 2.0
104 stars 82 forks source link

Handle time as a native type #42

Open ZhikuiRen opened 4 years ago

ZhikuiRen commented 4 years ago

Define a type specifier for times.
There are various units of times (seconds through microseconds) in different interfaces and it is going to cause an issue

wak-google commented 4 years ago

I actually have a commit for this somewhere to use chrono types for all the interfaces where durations are taken.

williamspatrick commented 4 years ago

@wak-google can you post this code in gerrit even if it is WIP? I might be able to finish it up.

wak-google commented 4 years ago

The commit I had was actually pretty trivial and doesn't keep the old function signatures which is probably not ideal for migration. This should probably be a 2 phase change.

https://gerrit.openbmc-project.xyz/c/openbmc/sdbusplus/+/18995

williamspatrick commented 4 years ago

@wak-google Thank you, but I think this is different than what was asked for in the issue (I only know this because I was involved in the code review where this came up). What @ZhikuiRen wanted us to do was to support in the YAML interface files a type of 'time' so that the dbus interfaces were not having to guess at what kind of conversion was needed on an integer property that happened to represent a time-interval.

wak-google commented 4 years ago

Oh right, yeah I haven't touched this at all. It would be nice if we could basically just implement a system that mirrors the c++, so signatures look like duration[uint64, micro] or time[uint32]

williamspatrick commented 4 years ago

Yes. I was considering simplifying it to just always use int64_t and then either always do things in micros on the dbus or allow time[duration] to be specified.

Do you see a good reason to allow types other than int64_t?