Make TIME field directly accessible

Bug #1182091 reported by Andrew Johnson
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Andrew Johnson

Bug Description

Users often request the ability to access the TIME field directly.
It could be provided as an array of 2 32-bit values, possibly with a specific string in ISO-8601 form say.

Tags: codeathon
Changed in epics-base:
assignee: nobody → Andrew Johnson (anj)
Revision history for this message
Jure Varlec (exzombie) wrote :

After discussion with Till Straumann and Freddie Akeroyd, we found the following usecases for this feature:

  - It's not intuitive that one can get the timestamp (e.g. using caget -a) by getting any field /except/ TIME.
  - Getting a 64-bit integer would simplify calculations.
  - Getting a double would also simplify calculations, albeit with reduced precision, and is better supported in EPICS. This would make it trivial to e.g. calculate time deltas using the calc record.
  - Getting two 32-bit integers is better supported than a 64-bit integer, but is a pain to use. Sometimes, one is only interested in seconds or only nanoseconds, in which case, CA filters could be used to get the desired value.
  - Writing to the timestamp: I'm sure there's someone out there who would like to have that, but it's not clear that it's a good idea and how it would interact with TSE and such.

We cannot change the underlying type of the TIME field as that would break the world. I have looked at two approaches: adding additional fields to dbCommon, and CA filters.

Adding an additional field could be done as follows:
  - Add a UINT64 field named TM64 (or something to that effect).
  - This field would contain a copy of the timestamp converted from the TIME field.
  - Handle the TIME field like fields with SPC_DBADDR are handled, except instead of calling cvt_dbaddr(), point to the data in TM64. This allows accesing the 64-bit timestamp through the TIME field.
  - Advantages:
    - Both TIME and TM64 are directly readable.
    - UINT64 will gracefully degrade to double and string: calc will work and caget will have full precision in ascii representation.
    - Additional fields can be added to serve seconds and nanoseconds separately.
  - Drawbacks:
    - No human-readable string (but I don't think that's a problem)
    - It introduces additional overhead for every single record. This is even more pronounced if more fields are added to provide seconds and nanoseconds separately.
    - I see no way to keep the timestamps in sync. One would need to wrap the rset::process() function, but that cannot be done because users outside of EPICS base may call it directly.

To implement this via CA filters, I propose extending the "ts" filter. It already has the right name, even though, given how general the name is, its current behaviour is oddly specific. It currently takes no arguments, so extending it shouldn't be a problem.

  - Seems easy to implement with no interference to other parts of the code.
  - Very flexible.
  - No overhead when not in use.
  - The TIME field itself would remain inaccessible. It was argued that one /can/ currently get the timestamp, but not in a straightforward way, and seems to me that CA filters are not yet part of users' muscle memory.

Revision history for this message
Jure Varlec (exzombie) wrote :

Another possibility I see for adding direct access to the TIME field while avoiding the drawback of keeping additional fields up to date is, like in the first proposal, to handle it similar to SPC_DBADDR. But instead of using a public field that would need to be updated after every processing, use a private filed inside dbCommonPvt. This field would only be updated when needed; as far as I can tell the appropriate time is whenever SPC_SBADDR is handled. Specifically, dbEntryToAddr() would, instead of calling the record's cvt_dbaddr(), convert epicsTimeStamp to UINT64 and return a pointer to that as DBADDR.

Does this make sense? The way I see it, that's the only way to access the value of the waveform, and thus should be sufficient as the only way to access the converted value of the timestamp. Is my understanding correct?

Revision history for this message
Andrew Johnson (anj) wrote :

Summing up our Zoom discussion: The ts post filter should be able to allocate a new db_field_log and convert the timestamp from the pLog->time field into a new value of whatever type/format was requested by the filter args.

My preference would be that ts:{fmt="%Y-%m-%d %H:%M:%S.%09f"} would return a DBF_STRING (although Michael isn't keen on accepting format strings from the network for security reasons; we can discuss that), while say ts:{epoch:1970} would return a double giving the number of seconds since 1970-01-01 00:00:00 UTC (that being the Posix epoch, while for 1990 it would use the EPICS epoch).

Revision history for this message
Ben Franksen (bfrk) wrote :

There should be a way to add special processing for fields in dbCommon that requires no updates to existing record supports. Then we could add a "virtual" field to dbCommon that automagically converts the TIME data to DBF_UINT64 on access. The mechanism to implement that is simple: instead of calling the record support's special method directly, first call the one for dbCommon; check via return value if that handled the field, otherwise invoke the record support's special.

Revision history for this message
Jure Varlec (exzombie) wrote :

Ben, IIUC your proposal is very similar to what I wrote above, except that I would have added the "virtual" aspect to TIME field itself. But in the end, it seems we can have our cake and eat it too: we can implement this with a channel filter and have the resulting flexibility, while at the same time treating the ".TIME" part of the PV name as special syntax that creates a channel filter (which is already the case for "[n]" for arrays or "$" for long strings).

Revision history for this message
Ben Franksen (bfrk) wrote :

Ah, yes, I agree that "virtualizing" the TIME field directly is even better.

I am not convinced of the filter solution if it implies yet another ad-hoc addition to channel name parsing to support `caget xx.TIME`. I understand the temptation but I recommend against it. There is no easy way to make such ad-hoc syntax extensions in a modular way and the more we add the harder it gets to make the next change. This approach clearly doesn't scale in the long run. I find it cleaner to add a special processing phase for dbCommon fields. Once this is implemented it can easily be re-used for other extensions.

Also, converting timestamps to human readable (or any other) formats is clearly a task that clients are better prepared to do; IMO we should not bother the server with that.

Revision history for this message
Freddie Akeroyd (freddie-akeroyd) wrote :

A string representation could be useful, but having the server return it in any format may be overkill. How about just allowing ts:{fmt="ISO8601"} or ts:{fmt="ISO"} ? Do we need to include the "T" for strict 8601 conformance, and is adding the timezone useful too?

Revision history for this message
Jure Varlec (exzombie) wrote :
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.