QCanBusFrame timestamp
-
I heavily use QCanBusFrame in my code. I'm actually reading CAN even from interfaces not supported by the CANbus system now in QT (partly because large chunks of my code predate that interface). When I manually fill out the timeStamp field I tend to only use the microSeconds portion. This is because the TimeStamp class seems to be very oddly coded and supports nearly nothing. For instance, what happens if you compare two timestamps? You can't. They're classes with multiple members. You can't compare it. You have to do comparisons by doing something like multiplying the seconds field by one million and adding the microseconds field to that. Now you have the # of microseconds which is a 64 bit integer and you can compare those. Or, since my manual filling out of the object uses just the microseconds field I can just directly compare microseconds and ignore the seconds field.
Also, there's a fromMicroSeconds function but no toMicroseconds() function. How do such obvious features get missed? Am I just too picky? These things seem to be extremely basic but they're just plain not there. It seems to me that the implementation was done very quickly and not with proper thought about what might be needed of the timestamp class.
Also, just a gentle note that 2^64 microseconds is about half a million years so there was no reason at all to have two 64 bit integers here. We don't need to send CAN frames 20 million years into the future. I think all of our code will be gone in 500,000 years. I guess I just completely don't understand the logic behind the whole TimeStamp implementation. Am I missing something? The normal QT CAN drivers do properly fill out seconds and microseconds but it's totally unnecessary to separate them. Doing so is only creating difficulties that didn't need to exist. If timeStamp had just been an int64_t it would have been 100x more useful.
-
@CollinK80 said in QCanBusFrame timestamp:
How do such obvious features get missed? Am I just too picky?
Ummm...yeah, you're being too picky, probably from a lack of familiarity with network or posix timesource programming. splitting the time values by seconds and fractional seconds is a legacy things that's been around forever. So no, the timestamp class really dones't need anything more...in fact it could have just as easily been declared as a struct like the corresponnding C structs used with clocks and been just fine....but of course that would break the "Qt way" of total encapsulation.
And why are you putting anything into the timestamp? if i read the docs correclty, then it should be automatically populated by the system when a frame is read.
-
Hi @CollinK80,
I guess you are right, the current timestamp is overengineered.
However, if we decide to change that, it must happen soon before QtSerialBus joins the Qt 6 package. We will still break customers using their own CAN plugins, but after all I think we can get a simpler solution and that should satisfy everyone :)
Would you like to help modeling and reviewing a new class?
I guess the timestamp was modeled after Linux
struct timeval(the SocketCAN plugin was the first one), but withquint64there is no real need for separate seconds and microseconds.And why are you putting anything into the timestamp? if i read the docs correclty, then it should be automatically populated by the system when a frame is read.
He has his own hardware layer and just re-uses
QCanBusFrame. So he has to fill the fields when receiving a frame.Regards
-
I've created QTBUG-87343 to track this suggestion. Please comment and vote there.
Thanks and regards
