Skip to content

Added timestamp struct based vspec, and posix timestamp as unit (#1)#884

Open
ali-momin12 wants to merge 4 commits intoCOVESA:masterfrom
ali-momin12:feature/struct_based_timestamp_addition
Open

Added timestamp struct based vspec, and posix timestamp as unit (#1)#884
ali-momin12 wants to merge 4 commits intoCOVESA:masterfrom
ali-momin12:feature/struct_based_timestamp_addition

Conversation

@ali-momin12
Copy link

  • Added timestamp struct based vspec, and nanosecond as unit in units.yaml
  • This change is a pre-requisite to the issue 878 which then as all the concerned stakeholders agreed upon as an enhancement in vss-tools (an upcoming PR in vss-tools repo)

@ali-momin12 ali-momin12 force-pushed the feature/struct_based_timestamp_addition branch from 9be1917 to 170f206 Compare March 2, 2026 02:22
@erikbosch
Copy link
Collaborator

At least for me the diff seems a bit strange, only highlighting changes already on master. I think I have seen similar results in the past sometimes when there have been a merge/rebase on the source branch which makes github confused. Not sure if a rebase and force push of source/head branch can help

@ali-momin12
Copy link
Author

At least for me the diff seems a bit strange, only highlighting changes already on master. I think I have seen similar results in the past sometimes when there have been a merge/rebase on the source branch which makes github confused. Not sure if a rebase and force push of source/head branch can help

Apologies for the confusion due to rebase (i was getting the DCO check failed due to one commit not being signed off and after rebasing, the confusion happened)

I have fixed this, kindly review whenever you can. thanks.


#
# Identifier
# Based on OAuth 2.0. Subject is the UserID inside the claim of the Issuer Domain.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a copy paste error, right?

@erikbosch
Copy link
Collaborator

Thanks for the PR, we will bring it up at the next VSS meeting.

As this would be the first time we would enter datatypes to the standard catalog (even if not used for any signal in catalog) we need to align on how to handle it. I would like to see that the file actually is used in tests, and one potential idea is to create a file VehicleDataTypes.vspec that includes the new file:

VehicleDataTypes:
  type: branch
  description: Top-level branch for vehicle data types.

#include include/Timestamp.vspec VehicleDataTypes

If we would do it like that we could extend our Makefile so that the file is included in one or more of our tests, possibly running like below.


(.venv-vss) erik@vbox:~/vehicle_signal_specification$ vspec export json -u ./spec/units.yaml --strict -s ./spec/VehicleSignalSpecification.vspec -t spec/VehicleDataTypes.vspec -o hej.json
[14:28:58] INFO     Loaded 'VSSQuantity', file=/home/erik/vehicle_signal_specification/spec/quantities.yaml, elements=30                        units_quantities.py:30
           INFO     Loaded 'VSSUnit', file=/home/erik/vehicle_signal_specification/spec/units.yaml, elements=69                                 units_quantities.py:30
           INFO     VSpecs (Types) loaded, amount=2                                                                                                       vspec.py:130
           INFO     Dynamic datatypes added=1                                                                                                              main.py:139
           INFO     VSSNode('.VehicleDataTypes.Time_t', data=VSSDataStruct(fqn='VehicleDataTypes.Time_t', type=<NodeType.STRUCT: 'struct'>, description='A tree.py:574
                    point in time', comment=None, delete=False, deprecation=None, constUID=None, fka=[], instantiate=True))                                           
           INFO     VSpecs loaded, amount=90                                                                                                              vspec.py:130
           INFO     Generating JSON output...                                                                                                              json.py:100
           INFO     Adding custom data types to signal dictionary   

# Based on OAuth 2.0. Subject is the UserID inside the claim of the Issuer Domain.
#

Time_t:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the _t suffix? Because of the posix C header? I would just name it Timestamp.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in the commit effbca6

type: struct
description: A point in time

Time_t.t_sec:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The t_ prefix also seems unnecessary. What about seconds and nanos?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in the commit effbca6

type: property
unit: unix-time
datatype: int64 # seconds (may use sentinel negative values)
description: time measured in seconds
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a hint to Unix time? If that's what we even want to express?

Something like "Unix epoch seconds"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in the commit effbca6

type: property
unit: ns
datatype: int64 # nanoseconds in [0, 1e9]
description: time measured in nanoseconds
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Nanosecond offset" maybe?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in the commit effbca6

erikbosch and others added 4 commits March 3, 2026 02:11
Based on COVESA/vss-tools#484 we now require unique unit names.
This gives problems here. We could alternatively remove the old mpg unit.

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
Signed-off-by: Ali Momin <ali_momin@jp.honda>
Signed-off-by: Ali Momin <ali_momin@jp.honda>
Signed-off-by: Ali Momin <ali_momin@jp.honda>
Updates done to timestamp.vspec contents as per the feedback.

---------
Signed-off-by: Ali Momin <ali_momin@jp.honda>
@ali-momin12 ali-momin12 force-pushed the feature/struct_based_timestamp_addition branch from 51ecedc to effbca6 Compare March 3, 2026 02:11
@ali-momin12 ali-momin12 requested a review from sschleemilch March 3, 2026 02:14
Timestamp.nanoseconds:
type: property
unit: ns
datatype: int64 # nanoseconds in [0, 1e9]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a math expert, but shall range possibly be described as [0, 1e9), i.e. max value is 999999999 nanoseconds.

We support min/max also for properties, found this example in vss-tools, so possibly we could specify min and max explicitly. Technically a uint32 would suffice right? is int64 proposed because some other framework or std definition use int64 for the nanosecond part.

NestedStruct.x:
  type: property
  description: "x property"
  datatype: double
  min: -10

NestedStruct.y:
  type: property
  description: "y property"
  datatype: double
  max: 10

@erikbosch
Copy link
Collaborator

MoM:

@sschleemilch
Copy link
Collaborator

Agreed that it should be in the type tree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants