eclipse-kuksa / kuksa-can-provider

Apache License 2.0
3 stars 10 forks source link

buf-fix : get_default_values issue #17

Closed jesseforimse closed 6 months ago

jesseforimse commented 6 months ago

When I tried to use kuksa-can-provider with vss2dbc, I got a problem with getting default dbc valules. Checking the code , I found that get_default_values function has a problem. I think this code should be changed like below.

Please check this code , and please give me your feedback.

Author: Jesse jesse.jung@ms-global.com Date: Mon Mar 25 16:10:31 2024 +0900

fix issue about getting default dbc value

diff --git a/dbcfeederlib/dbc2vssmapper.py b/dbcfeederlib/dbc2vssmapper.py index 12400c1..3e6cd72 100644 --- a/dbcfeederlib/dbc2vssmapper.py +++ b/dbcfeederlib/dbc2vssmapper.py @@ -512,7 +512,7 @@ class Mapper(DBCParser): res = {} for signal in self.get_signals_by_frame_id(can_id): if signal.name in self._dbc_default:

erikbosch commented 6 months ago

Thanks for reporting this. This is a regression from https://github.com/eclipse-kuksa/kuksa-can-provider/pull/9, unfortunately we do not have any test that detected this problem. You solution works.

Are you interested in creating and submitting a pull request? If not I could create one.

jesseforimse commented 6 months ago

Now I'm developing a prototype with eclipse kuksa that is a kind of conected service for a car. So, I am interested in this repository. So if you give me the authority for pull request and commit rules , I will do that with joy.

erikbosch commented 6 months ago

Great!

So what is needed in short is:

jesseforimse commented 6 months ago

When I tried to create a PR following to your guide, github said "Review required" . But I don't know how to add reviewer in this case. Because "Reviewers" isn't activated ,I can't click it. Actually It is a first time for a contributor, So, I hope you help me in detail.

erikbosch commented 6 months ago

So in Github only individuals with "triage" or "write" rights can add reviewers. In Eclipse Kuksa (and most other Eclipse projects) that means that some project committer must notice the PR and add reviewers, or review it themselves. I added myself and @lukasmittag. I also triggered the CI checks. The checks pass and as I tested the PR the other day I will also approve it.

This PR is trivial so I could have opted for directly merging it, but leave it open for a short period so that Lukas and others will have time to review as well, if they would like

jesseforimse commented 6 months ago

I got it.

erikbosch commented 6 months ago

As it is trivial and Lukas did not gave any comments I have merged the PR and will close the issue. The next time you will create a PR for this repo (and possibly all eclipse-kuksa repos) continuous integration actions will be triggered automatically for any subsequent PRs. Thanks for your contribution.