Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reading strings fields in structures on Omron NJ #431

Open
exzombie opened this issue Jul 20, 2023 · 7 comments
Open

Reading strings fields in structures on Omron NJ #431

exzombie opened this issue Jul 20, 2023 · 7 comments

Comments

@exzombie
Copy link

Looking at the code, the plc_tag_get_string() function appears to assume that the first two bytes at the provided offset contain the string length. This does not appear to always be the case for Omron NJ. When the tag is MyStruct.StringField and plc_tag_get_string() is called with offset=0, the entire string is returned. However, when the tag is MyStruct and plc_tag_get_string() is called with the byte offset of the field, the first two characters are missing. Calling plc_tag_get_raw_bytes() with the same offset returns the entire string.

The Omron manual documents that when an array of strings is fetched, the array elements will not be prefixed by size. The manual does not discuss the strings that are part of structures, but it would seem that the same applies. It seems to me that the fix, then, would be to only read the size if the offset provided to plc_tag_get_string() is zero.

In #430, you mentioned that Omron support is a contribution from a while ago. Is it possible that strings as structure and array members were simply not tested at the time? Or could it be that this is something that changed with time?

@timyhac
Copy link

timyhac commented Jul 20, 2023

There are some attributes in the tag string that should be able to configure this: https://github.com/libplctag/libplctag/wiki/Tag-String-Attributes#optional-generic-string-attributes

It looks like in the case of Omron, str_is_counted defaults to true but you suggest should default to false - can you link to the source documents?

@exzombie
Copy link
Author

Not really. String tags are counted. It's just strings in structures and arrays that are not. For arrays, this is described in section 8-7-4 of EtherNet/IP Port User's Manual. The explanation sucks: there is insufficient detail, structs are not discussed, and the examples have errors (?? invalid data, really? I'm pretty sure strings are null-terminated)

I don't have a string array to test with right now, so I can't say for sure what the byte order is. But what I can say for sure is

  • The tag MySingleString refers to a counted string.
  • The tag MyStringArray[1] refers to a counted string.
  • The tag MyStringArray contains non-counted strings.
  • The tag MyStruct.StringMember refers to a counted string.
  • The tag MyStruct contains a non-counted string, so calling plc_tag_get_string() with the offset of the string member yields garbage.

@kyle-github
Copy link
Member

Thanks for using the library, and many thanks for that link!

I am reading through that doc and am absolutely horrified that one of their network examples has a PLC with a direct connection to a router on the open Internet! I did not see any text about, "please use this configuration so that high-school script kiddies can take over your factory!" Globally routable IP addresses on a PLC?

At any rate... back to strings. The list above is disheartening. Is Omron really that complicated? That is not ideal. While users can add the string attributes for each individual tag, that is painful :-(

At this point, that's about all I can do: document what you have above with sample tag attributes. Yuck.

@ptsOSL
Copy link

ptsOSL commented Mar 5, 2024

So I am also trying to read strings from an OmronNJ and as discussed, strings which are accessed individually are transferred quite differently from strings embedded inside of a structure. Focusing on addressing an individual string of maximum length 60 within a structure such as struct.string1, I have had some success writing to these with the following attributes (and a minor change to libplctag):

str_is_zero_terminated=0
str_is_fixed_length=0
str_pad_bytes=0
str_is_counted=1
str_count_word_bytes=2
str_max_capacity=60

I have however had two additional problems. Firstly, the amount of data returned depends on the amount of characters stored in the string, not the maximum string length. Perhaps there is a better way, but to write to the string, I have had to set the tag buffer size to 60 to ensure there is enough capacity to write a full string. (I believe by default the size is set to the size read from the string in the PLC? but the new string which the user may choose to write may be larger than this value.) Then I have set the string which is usually much shorter than the capacity, before resetting the tag buffer size to the the correct size for the string I have written to it, before I actually write to the PLC. This is required so that the string doesn't have a bunch of trailing zeroes which the PLC sees as being extra data beyond the string length specified at the start of the string.

The second problem is more prohibitive, my write messages are getting padded with a null character if not multiples of 16-bits. I think this padding is being added as part of the CIP standard? (rather than because it is a string, as I have defined str_pad_bytes=0). Anyways, commenting out the if statement starting on line 1090 in eip_cip.c (build_write_request_connected()) fixes this, but is only a temporary solution. This changes the CIP message from:

d0 00 01 00 03 00 61 62 63 00 (returns error too much data)
into
d0 00 01 00 03 00 61 62 63 (succeeds)

Let me know if I am misunderstanding something, but if changes do need to be made for Omron NJ specifically, I am happy to be involved and I have an OmronNJ to test with for the next 3 months or so. Cheers

@kyle-github
Copy link
Member

Thanks for testing this! Very helpful. Do you know if stand-alone strings behave the same was as strings in a UDT/struct?

The padding is (IIRC, it has been many years) required on some Rockwell PLCs.

The library has no idea of what the actual size of the string is. The PLC only sends back the valid bytes of the string. Where the library could be easier to use is if it automatically resized the buffer when you tried to write a larger string. But, without knowing the actual size of the string, that might result in an error from the PLC.

I noticed that you use no zero-termination. The base definition for strings for Omron that the library uses does set zero-termination to true. Is that wrong?

@ptsOSL
Copy link

ptsOSL commented Mar 5, 2024

Thanks for the explanation and quick response. I cant currently test reading a standalone string, only an embedded string either by reading the UDT or addressing the field. However I can say that whether the string is zero-terminated changes between the two. When addressing an embedded string directly, you are given a 2-byte string size followed by the string and no zero-termination. The amount of characters sent is the amount which are actually written in the PLC and this does not include null bytes to match the maximum size of the string

When requesting the entire structure, you get no string size, but you do get a zero-termination and you also recieve the null bytes to pad out the maximum length of the string, if it is shorter than the max.

I dont know if this actually helps, but I will include an example of the command specific data from wireshark for reading an embedded string from a UDT vs reading the whole UDT:

Where the UDT is defined as: String[60], String[40], Real, Real, Real, Real, Real, UINT, BOOL, String[40], String[40]

Field/child string datatype:
0070 d0 00 3b 00 74 65 73 74 73 74 72 69 ..;.teststri
0080 6e 67 31 74 65 73 74 73 74 72 69 6e 67 31 74 65 ng1teststring1te
0090 73 74 73 74 72 69 6e 67 31 74 65 73 74 73 74 72 ststring1teststr
00a0 69 6e 67 31 74 65 73 74 73 74 72 69 6e 67 31 74 ing1teststring1t
00b0 65 73 74 est

UDT:
0070 a0 02 68 0a 74 65 73 74 73 74 72 69 ..h.teststri
0080 6e 67 31 74 65 73 74 73 74 72 69 6e 67 31 74 65 ng1teststring1te
0090 73 74 73 74 72 69 6e 67 31 74 65 73 74 73 74 72 ststring1teststr
00a0 69 6e 67 31 74 65 73 74 73 74 72 69 6e 67 31 74 ing1teststring1t
00b0 65 73 74 00 74 65 73 74 73 74 72 69 6e 67 32 74 est.teststring2t
00c0 65 73 74 73 74 72 69 6e 67 32 74 65 73 74 73 74 eststring2testst
00d0 72 69 6e 67 32 74 65 73 74 73 74 00 8b 6c 87 3f ring2testst..l.?
00e0 00 80 3b 44 00 00 7a 44 00 00 7a 43 00 00 00 00 ..;D..zD..zC....
00f0 01 00 01 00 41 44 65 73 63 5f 31 00 00 00 00 00 ....ADesc_1.....
0100 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
0110 00 00 00 00 00 00 00 00 00 00 00 00 45 67 75 00 ............Egu.
0120 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
0130 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
0140 00 00 00 00

Ill let you know when I can test a stand-alone string.

@ptsOSL
Copy link

ptsOSL commented Mar 20, 2024

I've done a bit more testing on reading strings...

Requesting a stand-along string returns a string which is not null terminated but is counted (2 bytes). The size returned is based on the number of characters written in the string, not the actual size of the string. My test returned d0 00 04 00 74 65 73 74. This is the same as when I access an individual string member of an array or struct.

I have tried to read a slice of an array of strings but have only received CIP status code 0x20 (8018 - zero elements or out of range data requested) when attempting to do this. I'm not sure why, trying to access an INT array with the same settings works fine. I dont think this is a problem with libplctag.

I have also been unable to access an entire string array tag, when attempting to do this I receive CIP status code 0x1F (8007) which suggests "An inaccessible variable was specified". Which is odd as the manual states that this is possible. Again, I dont think libplctag is at fault.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants