Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend existing implementation of DDSIDL exporter to support struct types. #262

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

kkoppolu1
Copy link
Collaborator

@kkoppolu1 kkoppolu1 commented Apr 4, 2023

Confirm expected output for DDSIDL before proceeding with the implementation

  • Each struct in vspec is translated to corresponding struct in DDSIDL.
  • The struct name is same as the base name of the node
  • The qualified name of the struct becomes a module

@erikbosch
Copy link
Collaborator

erikbosch commented Apr 4, 2023

Discussion:

  • Krishna: Proposal in PR
  • Sebastian - if we find a better way that is possibly ok
  • Neil to check, will prepare draft PR with some proposals
  • Adnan: We will not always have the possibility to generate something directly useful, but only parts. Do we see API as parts of what VSS tools shall generate
  • Erik: In long run good to document how the tools is supposed to work and how output is supposed to be used
  • Neil: Good to document how output shall look like
  • Please review until next week, focus on format of generated IDL

@erikbosch
Copy link
Collaborator

Meeting notes:

  • Neil will upload PR with doc on how DDSIDL output shall look like
  • Best if structs are included in that doc
  • When document is reviewed then then this PR can be reviewed
  • Krishna: Is DDS considered a first-class exporter, i.e. must structs in dds work for release
  • Sebastian: Do not see it as blocker
  • Krishna: Can we see others like binary as "feature extensions as well"
  • Ulf: Will not have time for binary
  • Erik: Ok for everyone to release VSS 4.0 even if not all exporters support it?
  • Krishna: Do we have any graphql experts
  • Adnan: We will provide something to GraphQl exporter.

@neil-rti
Copy link

Opened PR to address the use of struct, modules, and signal groupings via the use of new keywords in VSS, and by creating a format specifying document for each exporter type in vss-tools. COVESA/vehicle_signal_specification#576
Looking forward to the discussions.

@SebastianSchildt
Copy link
Collaborator

Meeting 08/15

Check whether a concept like COVESA/vehicle_signal_specification#576 will be impkmented in the ner/midterm. If yes, we shold think about struct support on top of that approach, if not let's see if the approach here can be reabsed, made work based on the existing DDS exporter with reasonable effort.

@kkoppolu1
Copy link
Collaborator Author

Meeting 08/15

Check whether a concept like COVESA/vehicle_signal_specification#576 will be impkmented in the near/midterm. If yes, we should think about struct support on top of that approach, if not let's see if the approach here can be rebased, made work based on the existing DDS exporter with reasonable effort.

@neil-rti is overlay approach described in the above PR being worked on?

@erikbosch erikbosch added the Status:WorkInProgress Someone is actively working on changing/extending this PR label Sep 20, 2023
@kkoppolu1 kkoppolu1 force-pushed the kkoppolu/ddsidl branch 5 times, most recently from 321b795 to 2462c9b Compare September 23, 2023 23:50
@kkoppolu1
Copy link
Collaborator Author

kkoppolu1 commented Sep 23, 2023

Re-based the changes. Example output is below. Once output format is agreed upon, I'll write the unit tests.

Re-cap of the output rules:

  1. Each struct in vspec is translated to corresponding struct in DDSIDL.
  2. The struct name is same as the base name of the node
  3. The qualified name of the struct becomes a module
    vspec inputs:
    https://github.com/COVESA/vss-tools/blob/master/tests/vspec/test_structs/test.vspec
#
A:
  type: branch
  description: Branch A.

A.UInt8:
  datatype: uint8
  type: sensor
  unit: km
  description: A uint8.

A.ParentStructSensor:
  datatype: VehicleDataTypes.TestBranch1.ParentStruct
  type: sensor
  unit: km
  description: A rich sensor with user-defined data type.

A.NestedStructSensor:
  datatype: VehicleDataTypes.TestBranch1.NestedStruct
  type: sensor
  unit: km
  description: A rich sensor with user-defined data type.

Types are defined at:
https://github.com/COVESA/vss-tools/blob/master/tests/vspec/test_structs/VehicleDataTypes.vspec

Example output:

module VehicleDataTypes {
module TestBranch1 {
struct NestedStruct {
double x;
double y;
double z;
};
struct ParentStruct {
VehicleDataTypes::TestBranch1::NestedStruct x_property;
VehicleDataTypes::TestBranch1::NestedStruct y_property;
sequence<VehicleDataTypes::TestBranch1::NestedStruct> x_properties;
sequence<VehicleDataTypes::TestBranch1::NestedStruct> y_properties;
double z_property;
};
};
};
module A
{
struct _UInt8
{
octet value;
//const string unit="km";
//const string type ="sensor";
//const string description="A uint8.";
};
struct ParentStructSensor
{
VehicleDataTypes::TestBranch1::ParentStruct value;
//const string type ="sensor";
//const string description="A rich sensor with user-defined data type.";
};
struct NestedStructSensor
{
VehicleDataTypes::TestBranch1::NestedStruct value;
//const string type ="sensor";
//const string description="A rich sensor with user-defined data type.";
};
};

@erikbosch
Copy link
Collaborator

erikbosch commented Sep 26, 2023

Meeting notes:

  • Please review, ambition is to decide to "lock" output at next meeting so that Krishna can work on unittests
  • Krishna to add example (source vspec plus result) to make it easier to understand mapping

@erikbosch erikbosch added the Status:Review Please review/discuss contents label Sep 26, 2023
@erikbosch
Copy link
Collaborator

Meeting notes:

  • Output format accepted, OK for Krishna to start with unit tests

@kkoppolu1
Copy link
Collaborator Author

kkoppolu1 commented Oct 9, 2023

Test plan update:
The file vspec2ddsidl does not have any unit tests. The existing environment of vss-tools does not include idlc. Hence, we cannot verify in a unit test whether the idl compilation will succeed or not.

In a separate PR, we should introduce the idlc dependency and add unit tests to vspec2ddsidl

For this PR, I tested manually that the (python) code generation succeeds for cyclonedds

Test command for processing the IDL file generated (signals_out.idl):

idlc -l py ./signals_out.idl

Generated code:
Dir: VehicleDataTypes/TestBranch1
File: _signals_out.py

"""
  Generated by Eclipse Cyclone DDS idlc Python Backend
  Cyclone DDS IDL version: v0.11.0
  Module: VehicleDataTypes.TestBranch1
  IDL file: signals_out.idl

"""

from dataclasses import dataclass
from enum import auto
from typing import TYPE_CHECKING, Optional

import cyclonedds.idl as idl
import cyclonedds.idl.annotations as annotate
import cyclonedds.idl.types as types

# root module import for resolving types
import VehicleDataTypes


@dataclass
@annotate.final
@annotate.autoid("sequential")
class NestedStruct(idl.IdlStruct, typename="VehicleDataTypes.TestBranch1.NestedStruct"):
    x: types.float64
    y: types.float64
    z: types.float64


@dataclass
@annotate.final
@annotate.autoid("sequential")
class ParentStruct(idl.IdlStruct, typename="VehicleDataTypes.TestBranch1.ParentStruct"):
    x_property: 'VehicleDataTypes.TestBranch1.NestedStruct'
    y_property: 'VehicleDataTypes.TestBranch1.NestedStruct'
    x_properties: types.sequence['VehicleDataTypes.TestBranch1.NestedStruct']
    y_properties: types.sequence['VehicleDataTypes.TestBranch1.NestedStruct']
    z_property: types.float64

Dir: A
File: _signals_out.py

"""
  Generated by Eclipse Cyclone DDS idlc Python Backend
  Cyclone DDS IDL version: v0.11.0
  Module: A
  IDL file: signals_out.idl

"""

from dataclasses import dataclass
from enum import auto
from typing import TYPE_CHECKING, Optional

import cyclonedds.idl as idl
import cyclonedds.idl.annotations as annotate
import cyclonedds.idl.types as types

# root module import for resolving types
import A

if TYPE_CHECKING:
    import VehicleDataTypes.TestBranch1



@dataclass
@annotate.final
@annotate.autoid("sequential")
class UInt8(idl.IdlStruct, typename="A.UInt8"):
    value: types.byte


@dataclass
@annotate.final
@annotate.autoid("sequential")
class ParentStructSensor(idl.IdlStruct, typename="A.ParentStructSensor"):
    value: 'VehicleDataTypes.TestBranch1.ParentStruct'


@dataclass
@annotate.final
@annotate.autoid("sequential")
class NestedStructSensor(idl.IdlStruct, typename="A.NestedStructSensor"):
    value: 'VehicleDataTypes.TestBranch1.NestedStruct'


@kkoppolu1 kkoppolu1 changed the title DRAFT: First-cut impl of struct to DDSIDL Extend existing implementation of DDSIDL exporter to support struct types. Oct 9, 2023
@kkoppolu1 kkoppolu1 removed the Status:WorkInProgress Someone is actively working on changing/extending this PR label Oct 9, 2023
@kkoppolu1 kkoppolu1 marked this pull request as ready for review October 9, 2023 01:26
@erikbosch
Copy link
Collaborator

Meeting notes:

  • Please review, will be merged before next meeting if there are no comments

@erikbosch
Copy link
Collaborator

@kkoppolu1 - if you have time feel free to look on the conflicts. If you do not have time I can assist.

Signed-off-by: Krishna Koppolu <kkoppolu@amazon.com>
@erikbosch erikbosch merged commit 590d86e into COVESA:master Oct 23, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Review Please review/discuss contents
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants