feat(plc4j/eip): Report the information if the tag address is invalid - #2706
Open
andvasp wants to merge 1 commit into
Open
feat(plc4j/eip): Report the information if the tag address is invalid#2706andvasp wants to merge 1 commit into
andvasp wants to merge 1 commit into
Conversation
Contributor
Author
|
HI @chrisdutz. Do you believe it can be merged before release 1.0.0? |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates EIP read handling to report invalid tag addresses more accurately.
Changes:
- Centralizes CIP read-response conversion.
- Maps relevant CIP errors to
INVALID_ADDRESS. - Reuses response extraction for unconnected reads.
Suppressed comments (1)
plc4j/drivers/eip/src/main/java/org/apache/plc4x/java/eip/base/EipTcpConnection.java:1110
- The new status translation is the core behavior of this PR, but none of the EIP unit or driver tests exercises these error statuses. Add regression cases for
0x03,0x04, and0x05, including the forced-unconnectedCipConnectedResponseshape and a normalCipReadResponse; this would catch the response-type regression above.
if (status == CIPStatus.PathDestinationUnknown.getValue() ||
status == CIPStatus.PathSegmentError.getValue() ||
status == CIPStatus.InvalidParameterValue.getValue()) {
return PlcResponseCode.INVALID_ADDRESS;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
|
Hi @chrisdutz ! What about this PR. Can you review it? |
Contributor
|
I'll look into this as soon as I manage to free a few seconds of time ... I'm sort of working my butt off in preparing PLC4X for 1.0.0 and the release ... your PRs always claim to be simple, but when looking at them the reformatting you keep on doing makes them all but simple to spot the changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi @chrisdutz!
This is a simple PR to report tag INVALID_ADDRESS.
I discover this when analysing a problem where I am trying to read a valid tag address but the PLC does not recognize it. I am investigating it and probability I will create an issue.