search

ros-drivers / odva_ethernetip

Library implementing ODVA EtherNet/IP (Industrial Protocol).
94 stars 48 forks source link

Attribute id 2 bytes #7

Open andreucm opened 6 years ago

andreucm commented 6 years ago

This PR allows to operate with devices with 2-Byte long Attribute ID's, instead of just 1-Byte long attribute ID. Further details at commit comments. Thanks for reviewing it.

andreucm commented 6 years ago

Hi,

CI has done its job!

The build fails because there is an ambiguity with the headers of the added methods at Session class.

To solve this ambiguity , in our application code we call the set/getSingleAttributeX() methods passing always the argument "attribute_id" as a typed constant (either USINT or UINT), instead of passing a single number, which causes ambiguity since the compiler does not know if the number is USINT or UINT.

At path_test.cpp, as well as session_test, attribute_id arguments are passed as (untyped) numbers , so there is an ambiguity on which method to call.

An option could be to implement these methods with templates, but from my point of view this approach is not readable at all, since the template argument would be the type of argument_id, instead of being the type of return/input value of a get/set. Moreover I think it will also result in a backwards API break.

In summary, this PR causes a backward break with potential existing code, so I'm open to receive proposals on which could be the better way to accept 2-byte long attributes id, but without breaking old API. I'll carry on because I think this feature is important, otherwise there is a subset of EthernetIP devices that simply will not work.

Sorry for the inconveniences, inputs are welcome,

andreu

andreucm commented 6 years ago

Hi Mike,

as you suggested, I've implemented the overloading of <<, and removed the print() method. Anyway, this printing functionality was used just for debugging.

Now, we should focus on the core of this PR, which is to allow to work with devices with attribute_id's > 255, so requiring 2Bytes. Simco motor drive (Wittenstein) is an example of a device with attribute_id's in the range of [2300,3000]

In the commits of this PR, Set/GetsingleAttributeX() methods are implemented at Session allowing 2Byte long attributes. Path has been also modified to allow ethernetip paths with 2Byte long attributes.

The problem remains with tests with non-typed numbers (hardcoded numbers), which causes ambiguity between EIP_USINT and EIP_UINT versions. I modified all these tests locally, and they passed ok, but I didn't push them, since I know that this implementation could imply an API break with existing code elsewhere (e.g. omron_os32c), so I still wonder how to solve this issue.

thanks

andreu