Wrong hbyte in function public Byte[] getReadCommand()

[sclcwwl]

I wonder if some issue in the KRLVariable.java line121,131,134 for hbyte? byte hbyte = (byte) ((cmd.length >> 8) & 0xff00); byte lbyte = (byte) (cmd.length & 0x00ff);

hbyte will always be zero. if cmd.length > 0xff , this will lose data in hbyte.

I thinks it should be like below. byte hbyte = (byte) ((cmd.length >> 8) & 0x00ff); byte lbyte = (byte) (cmd.length & 0x00ff);

[markaren]

I don't know. Googled that solution some years ago (convert int to 2 bytes in java). Have you encountered any problems where you think this is the cause?

[Laith-S]

Can confirm! I also ran into this non-intrusive bug. It prevents correct setting of not only the hbyte of the ID, but also the variable and message lengths. It is as sclcwwl said caused by incorrect storage of the most significant byte of these values and in order to correct this the lines with the following structure for the ID, variable and message length:

(byte) ((*.length >> 8) & 0xff00); e.g. (byte) ((cmd.length >> 8) & 0xff00);

This should be replaced by :

(byte) ((*.length & 0xFF00) >> 8); e.g. (byte) ((cmd.length & 0xFF00) >> 8);

The bug effectively prevents the MSB to be set and will always be zero. However this is only critical for lengths greater than 0xFF (255). If not corrected anything with value or length greater than 0xFF will cause an overflow and cause KUKAVARPROXY to incorrectly decode it.

So basically if you have an id e.g. 257 it will be displayed as 01, since it overflowed

[markaren]

Does anyone mind putting up a pull request on this?

[Laith-S]

I will try to do that today. Also i want to add a method that lets you get the class that implemented KRLVariable to know what type of variable it is.

[markaren]

Could you make those separate requests? Also isn't getClass().getSimpleName() or instanceof sufficient?

[Laith-S]

yes. but when working with javaFX tables, i was not able to get the property directly Dx more specifically a new PropertyValueFactory

[markaren]

Should be fixed in latest commit