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

Need fmi structural parameter to choose the protocol: UDP/TCP #19

Closed
adriantirea opened this issue May 16, 2023 · 11 comments
Closed

Need fmi structural parameter to choose the protocol: UDP/TCP #19

adriantirea opened this issue May 16, 2023 · 11 comments
Assignees
Milestone

Comments

@adriantirea
Copy link

fmi-ls-xcp has until now three fmi3 structural parameters in ModelDesciption.xml:

org.fmi-standard.fmi-ls-xcp.EnableInternalXcpService
org.fmi-standard.fmi-ls-xcp.ListenIpAddress
org.fmi-standard.fmi-ls-xcp.ListenPortNumber

We need also:

    org.fmi-standard.fmi-ls-xcp.ProtocolType
        Description:  "IP protocol where the XCP slave listens for XCP protocol commands. One of TCP or UDP."
        Type:         String
        Causality:    structuralParameter
        Variability:  fixed
        Start:        "TCP"
@PTaeuberDS
Copy link
Collaborator

FMI Design Meeting Paderborn:
We do not need this parameter but we need the parameters "ListenPort" and (calculated optional parameter)"IpAddress" in both variants, for TCP and UDP:

org.fmi-standard.fmi-ls-xcp.ListenPortNumber
-> org.fmi_standard.fmi_ls_xcp.TcpListenPortNumber, org.fmi_standard.fmi_ls_xcp.UdpListenPortNumber

org.fmi-standard.fmi-ls-xcp.ListenIpAddress
-> org.fmi_standard.fmi_ls_xcp.TcpIpAddress, org.fmi_standard.fmi_ls_xcp.UdpIpAddress

Also the parameter org.fmi-standard.fmi-ls-xcp.EnableInternalXcpService should be renamed to org.fmi_standard.fmi_ls_xcp.EnableXcpService.

@PTaeuberDS PTaeuberDS self-assigned this May 24, 2023
PTaeuberDS added a commit to PTaeuberDS/fmi-ls-xcp that referenced this issue Jun 12, 2023
PTaeuberDS added a commit to PTaeuberDS/fmi-ls-xcp that referenced this issue Jun 12, 2023
@PTaeuberDS
Copy link
Collaborator

Layered Standard for XCP Web-Meeting (13.06.2023):
We only need to split each parameter for both protocols, if it is possible to have an XCP service that is communicating over both protocols at the same time.
Pierre: I think this is possible

@PTaeuberDS
Copy link
Collaborator

I addressed the question regarding whether the XCP service can communicate over both protocols TCP and UDP at the same time. I have not found any counterexample that this is not possible. However, implementing such functionality would introduce additional complexity to our layered standard.

We already figured out that, if the FMU supports both protocols and the importer wants to use only one of it, the port for the unused protocol could be set to -1 (if we define it as signed integer).

But the FMU should also be able to announce that it supports both protocols, but only exclusive and not at the same time, which is probably a more common scenario.

In this case, we would need another parameter or manifest attribute or would need to specifiy that it must be documented in the documentation folder what happens in such a case, e.g., that the XCP service always uses TCP if both ports have a valid port number.

This leads to much complexity for a probably very rare use case.

Personally, I still favor a variant introducing another variable for the protocol. If the XCP service is able to handle both protocols, it is a structuralParameter, with which TCP or UDP can be selected exclusively, otherwise it is a constant.

I think this is just a small limitation...

@pmai @andreas-junghanns
Please comment.

@andreas-junghanns
Copy link
Contributor

Personally, I still favor a variant introducing another variable for the protocol. If the XCP service is able to handle both protocols, it is a structuralParameter, with which TCP or UDP can be selected exclusively, otherwise it is a constant.

I think this is just a small limitation...

@pmai @andreas-junghanns Please comment.

I agree - I know of no implementation to mix TCP/UDP channel usage within the same XCP session - what would be the purpose? Therefore, supporting the election of a single active channel seems good to me and only a theoretical limitation.

@pmai
Copy link
Collaborator

pmai commented Jun 16, 2023

I still think we should not worry about fixing problems in A2L/XCP: If an A2L file provides multiple transports, it is always unclear whether they can be used in parallel or not (i.e. if the entity is multi-session capable). The only reason we provide the parameters is to allow for dynamic port assignments. It is not intended to otherwise fix gaps in A2L/XCP that tools will have to deal with anyhow (if a connection fails, it fails, whether due to not supporting multiple sessions or for other reasons; the tooling will have to handle it).

So I'm fine with the current definitions. On the other hand if people want to change to the other option, I can certainly live with this, it just seems a bit tedious and does not really fix the issue for most users, since they have to deal with non-FMU XCP slaves anyway.

@PTaeuberDS
Copy link
Collaborator

PTaeuberDS commented Jun 16, 2023

I added the variant with the duplicates for XCP multi-sessions to the open PR #26. I also created a new PR (#27) that adds one commit on top, in which the duplication is removed and another configuration variable for the protocol is added.

The two variants can be compared directly. We should decide which way to go soon and merge the corresponding PR.

@PTaeuberDS
Copy link
Collaborator

Due to simplification reasons, PR #26 is merged now and #27 is reduced to the one additional commit that adds a protocol variable. So, if we decide to go for the variable duplication for TCP and UDP we are done here. When we decide to go for the variant with the new additional protocol variable we just have to merge #27.

@PTaeuberDS
Copy link
Collaborator

I closed the PR that adds another protocol variable (#27). It seems to be more than a theoretical use case to have several channels in parallel, so I'm convinced now that we should not restrict this with our standard.

To make it even more complicated:
With "XCPplus", which is another kind of the IF_DATA XCP and not considered by our standard yet, it is even possible to run several instances of the same transport layer. For example, an IF_DATA XCPplus could contain the following instances:

/begin XCP_ON_UDP_IP
    0x0100                                      /* transport layer version */
    8670                                        /* PORT */
    ADDRESS "10.60.150.54"                      /* IP ADDRESS */
    TRANSPORT_LAYER_INSTANCE "INSTANCE UDP0"    /* instance name */
/end XCP_ON_UDP_IP

/begin XCP_ON_UDP_IP
    0x0100                                      /* transport layer version */
    8671                                        /* PORT */
    ADDRESS "10.60.150.54"                      /* IP ADDRESS */
    TRANSPORT_LAYER_INSTANCE "INSTANCE UDP1"    /* instance name */
/end XCP_ON_UDP_IP

/begin XCP_ON_UDP_IP
    0x0100                                      /* transport layer version */
    8672                                        /* PORT */
    ADDRESS "10.60.150.54"                      /* IP ADDRESS */
    TRANSPORT_LAYER_INSTANCE "INSTANCE UDP2"    /* instance name */
/end XCP_ON_UDP_IP

/begin XCP_ON_TCP_IP
    0x0100                                      /* transport layer version */
    8670                                        /* PORT */
    ADDRESS "10.60.150.54"                      /* IP ADDRESS */
    TRANSPORT_LAYER_INSTANCE "INSTANCE TCP0"    /* instance name */
/end XCP_ON_TCP_IP

/begin XCP_ON_TCP_IP
    0x0100                                      /* transport layer version */
    8671                                        /* PORT */
    ADDRESS "10.60.150.54"                      /* IP ADDRESS */
    TRANSPORT_LAYER_INSTANCE "INSTANCE TCP1"    /* instance name */
/end XCP_ON_TCP_IP

/begin XCP_ON_TCP_IP
    0x0100                                      /* transport layer version */
    8672                                        /* PORT */
    ADDRESS "10.60.150.54"                      /* IP ADDRESS */
    TRANSPORT_LAYER_INSTANCE "INSTANCE TCP2"    /* instance name */
/end XCP_ON_TCP_IP

If we want to allow such A2L files/ XCP services, too, we would have to rethink the design of the configuration parameters again. We would need to define something like config structs and arrays of that, or similar.

It's also worth mentioning that XCPplus is not a new feature, it is already included in the XCP 1.2 standard (2013). I also talked to colleagues who are using XCPplus.

@PTaeuberDS PTaeuberDS added this to the future milestone Aug 30, 2023
@PTaeuberDS
Copy link
Collaborator

As discussed in the Design Meeting yesterday, I set the milestone for the discussion about "how to handle the XCPplus feature" to "future".

@chrbertsch
Copy link
Collaborator

@bmenne-dspace : I will check, if this is checked by #76

@chrbertsch
Copy link
Collaborator

This is about XCP+, @PTaeuberDS will create a new ticket and close this one.

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

No branches or pull requests

6 participants