Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Add DDS provider #61

Merged
merged 12 commits into from
Mar 29, 2023
Merged

Conversation

lukasmittag
Copy link
Contributor

No description provided.

@lukasmittag
Copy link
Contributor Author

@AkhilTThomas

@lukasmittag lukasmittag marked this pull request as draft February 28, 2023 15:17
@lukasmittag
Copy link
Contributor Author

tests are missing as of now. Will be added in another commit.

@lukasmittag lukasmittag force-pushed the feature/DDS_feeder branch 4 times, most recently from 3af806d to 0f0b0c0 Compare March 6, 2023 13:11
@lukasmittag lukasmittag marked this pull request as ready for review March 16, 2023 12:34
Copy link
Contributor

@erikbosch erikbosch left a comment

Choose a reason for hiding this comment

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

what I have done:

  • Reviewed the code
  • Succeeded running unit tests
  • Succeeded getting data sent to databroker (after changing import path as shown in comment)

Apart from the import (which either should be fixed, or someone else check that current setup works for them) the rest is minor items where not necessarily everything needs to be fixed.

Build the docker image for amd64 and arm64
Run the provided tests for dds feeder

Update kuksa_dds_feeder.yml

Add setup for build

Add separate build for arm64 amd64 to CI

fix typo in CI file

Fix typo in dds tests setup

Add generation of idls python modules

Start databroker instance for integration test

Update Dockerfile

Create make_executable.sh

Update make_executable.sh
Copy link
Contributor

@erikbosch erikbosch left a comment

Choose a reason for hiding this comment

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

Looks good, only some minors that we possibly could ignore.

Possibly we should also mention ddsprovider on top level README (https://github.com/boschglobal/kuksa.val.feeders/blob/feature/DDS_feeder/README.md)

I did start ddprovider natively as well as from docker and did test from dds2val directory and all succeeded.

@lukasmittag
Copy link
Contributor Author

Looks good, only some minors that we possibly could ignore.

Possibly we should also mention ddsprovider on top level README (https://github.com/boschglobal/kuksa.val.feeders/blob/feature/DDS_feeder/README.md)

I did start ddprovider natively as well as from docker and did test from dds2val directory and all succeeded.

@SebastianSchildt should we merge or would you like to see a config file?

@lukasmittag lukasmittag changed the title Add DDS feeder Add DDS provider Mar 24, 2023
@lukasmittag lukasmittag requested a review from erikbosch March 27, 2023 07:04
Copy link
Contributor

@erikbosch erikbosch left a comment

Choose a reason for hiding this comment

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

Some minor ones. Did a test run with Docker and it works as expected

@SebastianSchildt SebastianSchildt merged commit e620e9b into eclipse-kuksa:main Mar 29, 2023
@erikbosch erikbosch deleted the feature/DDS_feeder branch September 29, 2023 09:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants