Skip to content

Erpcgen py union type #100

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 9, 2020
Merged

Erpcgen py union type #100

merged 4 commits into from
Sep 9, 2020

Conversation

zuzy
Copy link
Contributor

@zuzy zuzy commented Jun 5, 2020

Fix erpcgen python inout, union, type bug

zuzy added 2 commits December 25, 2019 16:46
@zuzy
Fixed extra return in client.cpp, when generages callbacks with retur…
6a0f2a6 
…n value.

Enriched test_callbacks demo with some new callbacks.
@zuzy
<fix>: Fix erpcgen python client inout argument bug
439affd 
<body>:
Insert a judge of parameter.direction

Issue #98
@MichalPrincNXP
Copy link
Member

Hello @zuzy , thank you for this PR, please give me some time to review and evaluate. Thank you.

@MichalPrincNXP MichalPrincNXP self-assigned this Jul 14, 2020
@MichalPrincNXP MichalPrincNXP mentioned this pull request Jul 14, 2020
Closed
@MichalPrincNXP MichalPrincNXP removed their assignment Jul 14, 2020
@MichalPrincNXP MichalPrincNXP self-requested a review July 14, 2020 13:38
@MichalPrincNXP
Copy link
Member

Hello @zuzy , I have looked at the first commit of this PR and it seems to be OK. I agree with modifications of the callback test but it needs to be rebased to the current head of develop branch and to resolve minor conflicts in test_callbacks_server_impl.cpp.
As for the rest of commits which are python-related, I would ask @Hadatko for the review (consultation with Tomas).
Anyway, there are several changes to be done in these last two python-related commits:

  • test_length_py.yml, test_name_py.yml and test_union_py.yml need to be updated to pass erpcgen py tests
  • "LOG_DEBUG" in PythonGenerator.cpp needs to be replaced by "Log::debug"

@MichalPrincNXP MichalPrincNXP requested a review from Hadatko August 3, 2020 14:12
@MichalPrincNXP MichalPrincNXP self-assigned this Aug 3, 2020
@zuzy
<fix>: Generate python union & type bug
840488e 
<body>:
1. Move aliases into group. Incase of type struct/union when import other IDL file.
2. Delete spare Union definition.
3. Fix discriminator bug: when the discriminator is named by other parameter.
4. Value the union data in __init__, delete the data argument
5. Match test_length/name/union.yml
@zuzy
Copy link
Contributor Author

zuzy commented Aug 28, 2020

Hello @zuzy , I have looked at the first commit of this PR and it seems to be OK. I agree with modifications of the callback test but it needs to be rebased to the current head of develop branch and to resolve minor conflicts in test_callbacks_server_impl.cpp.
As for the rest of commits which are python-related, I would ask @Hadatko for the review (consultation with Tomas).
Anyway, there are several changes to be done in these last two python-related commits:

  • test_length_py.yml, test_name_py.yml and test_union_py.yml need to be updated to pass erpcgen py tests
  • "LOG_DEBUG" in PythonGenerator.cpp needs to be replaced by "Log::debug"

Hello @MichalPrincNXP @Hadatko , thanks for reviewing.
I ignored the test in erpcgen before. Now they get fixed.

@zuzy
<feat>: Some error in erpcgen/test/readme.md
2499b13 
<body>:
- Because of path problem, py.test in `./erpcgen` folder cannot pass the `test/test_redundant_definitions.yml` case. And the case gets passed while in `./erpcgen/test`.
@MichalPrincNXP MichalPrincNXP merged commit 4b9aedf into EmbeddedRPC:develop Sep 9, 2020
@MichalPrincNXP
Copy link
Member

Thank you, @zuzy
Michal

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

Reviewers

@MichalPrincNXP MichalPrincNXP MichalPrincNXP approved these changes

@Hadatko Hadatko Awaiting requested review from Hadatko

Assignees

@MichalPrincNXP MichalPrincNXP

Labels

enhancement

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zuzy @MichalPrincNXP