Fix tcp server/client request

[floroeske]

Pull request type

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?
Issue Number: #907

What is the new behavior?
Pretend eCAL::STcpHeader before the request data and set the payload size on the client. Read and decode the header and read until all request data has been received before executing the callback on the server.

Does this introduce a breaking change?

• Yes
• No

Other information

We might need to put some thought into timeouts. I haven't added any server side timeouts. There is something in the client, but I cannot confirm that it works.

[github-actions]

clang-tidy made some suggestions

[github-actions]

warning: Value stored to 'bytes_used' is never read [clang-analyzer-deadcode.DeadStores]

          bytes_used += remaining_bytes;
          ^

ecal/core/src/service/asio_server.h:102: Value stored to 'bytes_used' is never read

          bytes_used += remaining_bytes;
          ^

[FlorianReimold]

Hey, I just wanted to give you a short status update, since this PR has been open for so long. Short in short: this issue haunts us. The fix causes an incompatibility and there is no way around that. But obviously not fixing it also isn't an option. You have done a great job fixing it. We are currently investigating if we take it a little further from here (as we have to break compatibility anyways) and further future-proof the protocol, so we will never have to get incompatible again.

[floroeske]

Thanks for the update. I am interested what the incompatibility is. Maybe I can help.
I have been running this patch on our devices since I've created this pull request. So far I haven't had issues.

[FlorianReimold]

The incompatibility introduced now obviously is, that you added the necessary header. So old and new eCAL Apps cannot communicate with each other, as the new eCAL Apps need the mandatory header, while old eCAL Apps cannot even understand it.

For the future, I really would like to get an actual protocol involved. So this means:

• The header is extended to contain a header-length and a payload length at least, so it is possible to extend the header in the future.
• At the beginning, a handshake is performed, where server and client agree on a protocol version. So if we ever change anything, we have the possibility to detect and switch the protocol at runtime.

Basically, do exactly what I did also do for tcp_pubsub, which is used for the eCAL TCP Transport layer.

Another option however is, to not implement a custom protocol on raw TCP, but use some third party library underneath. The first idea for that would be to use gRPC, which is the most hyped RPC protocol / library at the moment. It comes with a set of advantages and disadvantages. I am currently in the process of exploring these, to make an educated decision.
But please feel free to share your thoughts about it here, if you like 😊

[floroeske]

True. It's been a while.

I just used this header because it was already used in the rpc response.

Otherwise I would do the same. Review the request and response headers. Add a protocol version field, a checksum and maybe some placeholders.

I think this is fine and standard practice.

I was thinking a little about something like a "no protocol header" backwards compatibility mode. It could be a config option. When receiving a new request, try to decode the "new versioned" header first e.g read the checksum, protocol version, payload size etc. and validate the header. If this fails and the compatibility mode is on, read until there is no more data and try decode the packet the "old" way. But I don't think it will work very well as we have no idea how much data to read and this how we got here in the first place.

I wonder if gRPC might be a bit heavy for eCAL. We experimented with it first and then settled on eCAL. There are some more lightweight libraries which might be an option zeromq comes to mind.

[FlorianReimold]

As I am currently in the progress of understanding the impact and possibility of using gRPC in eCAL, I would be really interested in hearing about your experiments.

Could you elaborate why you didn't use gRPC?

[floroeske]

Our comparison was a bit different though. We choose between eCAL and GRPC.

The main reasons for us to use eCAL were the distributed design the easy to use pub/sub interface, the option to use protobuf serialization and services and ecal_mon_gui with it's plugin interface. I have worked extensively with ROS in the past so eCAL felt familiar.

GRPC has sever reflection which works a bit like the ROS 1 master. Services can query where to find other services.
Also I think the stream interface is similar to pub/sub but it works slightly different and is a bit more coupled.

But GRPC brings other advantages. Encryption might be one.

[KanonWY]

In the case of compatibility with the old version, the problem of sticky packets at the TCP sender will always exist. This may be a problem for users choosing this library.

[rex-schilasky]

Fix integrated in PR #1072. @floroeske would be great if you could review and/or test this.