nimbuscontrols / EIPScanner

Free implementation of EtherNet/IP in C++
https://eipscanner.readthedocs.io/en/latest/
MIT License
233 stars 93 forks source link

Add Large Forward Open service #25

Closed nefethael closed 4 years ago

nefethael commented 4 years ago

Signed-off-by: Vincent Prince vincent.prince.fr@gmail.com

nefethael commented 4 years ago

Hi all,

This is my first contribution to EIPScanner, I'm not sure my PR takes care of everything (examples, doc, etc)?

Also, I only tried LFO feature with OpENer stack and wireshark (I don't own any CIP adapter equipment), so I'm not 100% confident about implementation.

First commit allows output assembly to change data without vector copy, maybe a better solution would be to provide something like this in IOConnection? Current solution is not threadsafe.

using SendDataHandle = std::function<std::vector<uint8_t>&(void)>;

Second commit allows creation of ConnectionManager object as another class member without causing a timeout on first forward open.

Last commit adds large forward open service and provides a new NetworkConnectionParametersBuilder class that takes care of serializing/deserializing network connection parameters. It can be used this way

        ConnectionManager connectionManager;

        ConnectionParameters parameters;
        parameters.connectionPath = {0x20, 0x04,0x24, 151, 0x2C, 150, 0x2C, 100};
        parameters.o2tRealTimeFormat = true;
        parameters.originatorVendorId = 342;
        parameters.originatorSerialNumber = 32423;

        parameters.t2oNetworkConnectionParams =
                NetworkConnectionParametersBuilder(0, true)
                .SetConnectionType(NetworkConnectionParametersBuilder::P2P)
                .SetPriority(NetworkConnectionParametersBuilder::SCHEDULED)
                .SetConnectionSize(32).Build();

        parameters.o2tNetworkConnectionParams =
                NetworkConnectionParametersBuilder(0, true)
                .SetConnectionType(NetworkConnectionParametersBuilder::P2P)
                .SetPriority(NetworkConnectionParametersBuilder::SCHEDULED)
                .SetConnectionSize(32).Build();

        parameters.originatorSerialNumber = 0x12345;
        parameters.o2tRPI = 1000000;
        parameters.t2oRPI = 1000000;
        parameters.transportTypeTrigger |= NetworkConnectionParams::CLASS1;

        auto io = connectionManager.largeForwardOpen(si, parameters);
        ...

I'm open to any remarks & suggestions

Best regards, Vincent

atimin commented 4 years ago

Hey @nefethael , thank you for the contribution.

We appreciate your help and we are open to work here together. Unfortunately, I'm pretty busy at the moment and cannot make the code review in details. I'll give you my feedback at the weekend.

Best regards, Aleksey

nefethael commented 4 years ago

@flipback I rebased PR branch with corrections. I just reverted setDataToSend stuff without any workaround for 0-copy vector, they're not that big regarding send period :)

atimin commented 4 years ago

Hi @nefethael, thanks I see. What about other two topics?: 1) LOG_TRACE 2) Code conversion

nefethael commented 4 years ago

@flipback I removed LOG_TRACE in notify function and fixed upper/lower-case issues, normally it should be ok

atimin commented 4 years ago

@nefethael I didn't notice it because I was waiting for an additional commit =) Okay, I'll take your PR. Thank you very much for your contribution again!