truncate strings

[ifTaylor]

Issue:

Strings were being written but not read correctly due to differences in format specifiers for multi-write strings (packed four bytes '<I') and unpacking them (unpacked single byte '<B'). Even without the dissonance, both formats could write more than 82 characters and could result in a 'UnicodeDecodeError.'

Troubleshooting Efforts:

• The observed data type of the strings was 0xa0, which is a struct. I am unsure of how to intentionally define this data type in Studio5000 and would have expected _parse_multi_read() to have unpacked type 0xda, string, from the stripped data. This is why a list of applicable string types was made.

• I tested custom string data types with longer character limits, but they returned an "Unknown error 255" in the response status. The Micro800 character limit is defined at 255, but its ASCII instructions have limits at 82 characters. I do not have access to a Micro800 currently for runtime testing.

• I attempted to find indications of the defined string character limit in the data from _parse_multi_read() but had no success.

Other Proposed Solution:

• One approach could be to simply truncate the value at _make_standard_string(). However, this will not accurately reflect what was actually written in the response value.

Reference:

• Here's Studio5000 info on strings. Page 13 shows how longer string can be created. Link to documentation.

Open ended:

• I was not able to answer how the plc was holding onto the invalid string. In Studio5000 the monitored value displayed was truncated in this fashion already, but read as the full value written. I would have liked to power cycle the plc when in this state to see if the full value is held in the cache and the plc will construct the truncated value on power up.

[TheFern2]

Thanks for the pr. Very nice details!

A few things to note:

• wouldn't [0][:82] give you 83 chars?
• this change needs to be tested against micropython pr (note for maintainers)
• need tests improved for strings at the moment Randomizer.String only creates strings from 1-82 char size.

I am thinking sending longer strings are held in the heap until some cleanup or power cycle occurs. Thinking in terms of microcontrollers it allocates heap of N size (hoping there's a limit otherwise if you send a super long string you could run out of ram), goes from the heap, then there's a check function to truncate to 82, and send it to persistent memory area.

You could do a test of sending a bunch of long strings and then watching ram increase % in the plc task monitor web gui. Cycle plc and ram % should go back to normal.

[kyle-github]

Note that strings differ in size in some other CIP-based PLC types. IIRC, Micro8x0 can have strings up to 255 characters long.

[dmroeder]

I agree with @TheFern2, nice details.

Unfortunately, I think this will mess up Micro800 strings. Strings in the Logix platform are handled oddly compared to other platforms. 4 bytes for the length, 82 bytes for characters, then 2 bytes wasted at the end. When you write, all 88 bytes are written, even if you only have a single character in your string.

As @kyle-github says, strings in Micro800, you define how many characters each string tag is. They can be from 1-255 characters. When you write, you write the length (1 byte) and the characters, no additional characters like on the Logix platform.

The fix should probably be applied to _make_standard_string() since this is called only for Logix strings. I have rewritten it to solve the problem, though I'll sit on it if you want to take a crack at it. (Micro800 will take a separate fix).

@TheFern2 slicing a list smaller than the slice won't impact the list. So [0][:82] won't change the list, you'd still have 1 character.

[ifTaylor]

Thanks for the feedback!

@dmroeder Would the defined string length be found in the CIP client data? I tried to examine incoming data at _parese_multi_read(), but I could not find the resources to figure out the buffer sections.

Also, I updated the PR to reflect the suggested logix patch.

@TheFern2 are you using Thonny and something like a pi-pico to test micropython? And watching the ram % increase in the plc task monitor web gui is a good idea.

@kyle-github I had not considered other CIP-based PLCs outside of the 5000 and 800 platforms.

[TheFern2]

@ifTaylor The micropython testing instructions are here this is tested against the micropython unix port. I usually use pycharm for developing, the test commands can be ran in the terminal within the project directory. That note was mainly for Dustin or me, since it is a bit involved to compile micropython. Unless you plan on contributing to the micropython side of things I am not expecting everyone to test against micropython, whoever merge the requests will do it.

Next items on the line for me are to provide instructions for using the micropython docker container and for testing with a device, but since devices are very limited in memory it could be challenging to run the test suite. Though I presume most plc guys don't know how to use docker so not sure if that would be helpful in the end. The other solution would be to provide compiled binaries in a repo.