You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@plc4x.apache.org by GitBox <gi...@apache.org> on 2023/01/08 13:53:56 UTC

[GitHub] [plc4x] nielsbasjes opened a new pull request, #732: fix(plc4j/modbus): Cleanup of ModbusTag

nielsbasjes opened a new pull request, #732:
URL: https://github.com/apache/plc4x/pull/732

   This is my proposed set of (what I think are) improvements for the ModbusTag code.
   Summary:
   - The equals and hashcode were incorrect because the name of the actual class also matters.
   - My take on reducing the confusion around the -1 offset and the code complexity around all of this:
     - Simplified the code for readability
     - There is now a getLogicalAddress which returns the address the user configured.
     - The getAddressString returns a string that parses (which was not the case) AND yields an identical new tag when parsed (which was not the case: was shifted by 1 most of the time).
     - A more extensive set of tests that verifies all of this and ensures all supported formats for all tags yield the correct tag that is identical regardless of the used format.
   
   Looking forward to your feedback.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [plc4x] chrisdutz merged pull request #732: fix(plc4j/modbus): Cleanup of ModbusTag

Posted by GitBox <gi...@apache.org>.

chrisdutz merged PR #732:
URL: https://github.com/apache/plc4x/pull/732


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [plc4x] nielsbasjes commented on pull request #732: fix(plc4j/modbus): Cleanup of ModbusTag

Posted by GitBox <gi...@apache.org>.

nielsbasjes commented on PR #732:
URL: https://github.com/apache/plc4x/pull/732#issuecomment-1374852494

   Usually I'm happy to contribute. 
   Right now I would like to focus on the actual application code I'm building (in Java).
   So currently I won't be digging in to the Go/C/Rust code.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [plc4x] chrisdutz commented on pull request #732: fix(plc4j/modbus): Cleanup of ModbusTag

Posted by GitBox <gi...@apache.org>.

chrisdutz commented on PR #732:
URL: https://github.com/apache/plc4x/pull/732#issuecomment-1374845489

   And just asking ... would you feel able to do the same for the PLC4Go and/or PLC4C implementation? We're trying to keep them as in-sync as possible.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [plc4x] chrisdutz commented on pull request #732: fix(plc4j/modbus): Cleanup of ModbusTag

Posted by GitBox <gi...@apache.org>.

chrisdutz commented on PR #732:
URL: https://github.com/apache/plc4x/pull/732#issuecomment-1374851509

   Thank you for those changes :-) 
   
   And I guess ... if you would like to get started in Plc4go, I would be happy to assist you ... PLC4C right now is probably not really worth the effort. It's a pretty "experimental" thing which might even be replaced by PLC4Rust once that's done.
   If you're not interested, no worries :-)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [plc4x] nielsbasjes commented on pull request #732: fix(plc4j/modbus): Cleanup of ModbusTag

Posted by GitBox <gi...@apache.org>.

nielsbasjes commented on PR #732:
URL: https://github.com/apache/plc4x/pull/732#issuecomment-1374851037

   I have put up some additional changes with a getAddressStringPrefix() method.
   
   About the C and Go code ... I have never written any Go code yet and the last time I touched C/C++ was > 20 years ago. At this point I don't even know the basics of the build systems that are used (I assume things have changed since 2003). 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org