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

Alt. 1: Duplicated IpAddress and Port variables for both protocols #26

Merged
merged 10 commits into from
Jul 6, 2023

Conversation

PTaeuberDS
Copy link
Collaborator

@PTaeuberDS PTaeuberDS commented Jun 13, 2023

This is the same PR as #27 but without one commit that is added to #27 and adds another configuration parameter for the protocol.
In this PR (#26) the IpAddress and port variables are duplicated for the different protocol types for the special case that an XCP service uses TCP and UDP channels in parallel.

@PTaeuberDS
Copy link
Collaborator Author

@andreas-junghanns @pmai
@ everybody interested

The PR is ready for review. Due to the open discussion in #19 there are two variants of this PR now (see PR description).

Copy link
Contributor

@andreas-junghanns andreas-junghanns left a comment

Choose a reason for hiding this comment

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

Thank you: great work!

@PTaeuberDS PTaeuberDS changed the title Handling Configuration Parameters and other Issues: Variant 1 Alt. 1: Duplicated IpAddress and Port variables for both protocols Jul 5, 2023
@PTaeuberDS
Copy link
Collaborator Author

@andreas-junghanns I merged your review PR from main into this PR. Can we also merge this one and make this the new baseline? Then we can decide with the other PR if we want to go for the other alternative afterwards.

Copy link
Contributor

@andreas-junghanns andreas-junghanns left a comment

Choose a reason for hiding this comment

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

Big step forward. I think "FMI 2" and "FMI 3" should be replaced with "FMI 2.0" and "FMI 3.0" as this is what we use elsewhere. I know you mean FMI 2.X, which is technically more correct, but it looks strange if this is the only FMI standard text (is it?) that uses these. Can be fixed later.

@PTaeuberDS PTaeuberDS merged commit 0258a56 into modelica:main Jul 6, 2023
1 check passed
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.

2 participants