search

eclipse-tracecompass / org.eclipse.tracecompass

Eclipse Trace Compass
https://eclipse.dev/tracecompass/
Eclipse Public License 2.0
13 stars 13 forks source link

Expose the clock properties in CTFTrace #155

Closed siwei-zhang-work closed 1 month ago

siwei-zhang-work commented 1 month ago

There is the need to get clock scale of the trace. This commit added the clock scale to the environment variables of the trace.

[Added] clock scale in getEnvironment() in CTFTrace [Changed] moved clock offset into getEnvironment() in CTFTrace [Deprecated] CtfTmfTrace.CLOCK_OFFSET, use CTFTrace.CLOCK_OFFSET instead.

Signed-off-by: Siwei Zhang siwei.zhang@ericsson.com

MatthewKhouzam commented 1 month ago

Hi Siwei, any thoughts on complementing getEnvironment with the frequency? It would require less code and no API changes.

siwei-zhang-work commented 1 month ago

Hi Siwei, any thoughts on complementing getEnvironment with the frequency? It would require less code and no API changes.

But the getEnvironment() refers to a specific env block in the metadata which doesn't include frequency. Also complementing getEnvironment() still requires exposing frequency from the CTFClock. I changed to adding the frequency in the getProperties(), and the frequency is exposed the same way as time offset. Does this seem cleaner? Otherwise I'll add the frequency in the getEnvironment().

MatthewKhouzam commented 1 month ago

getEnvironment currently refers to env but it means all data that is not in events. I would recommend using it for all static information, also I will add get environment to TmfTrace. Trace Event should have this for metadata too. Maybe we can get a second opinion, but I would not add a new api for this instead of enhancing the output.

siwei-zhang-work commented 1 month ago

getEnvironment currently refers to env but it means all data that is not in events. I would recommend using it for all static information, also I will add get environment to TmfTrace. Trace Event should have this for metadata too. Maybe we can get a second opinion, but I would not add a new api for this instead of enhancing the output.

Sounds good, done.