paulscherrerinstitute / StreamDevice

EPICS Driver for message based I/O
GNU General Public License v3.0
28 stars 42 forks source link

Datatype on vxWorks causing checksum failure #63

Closed LeeHudsonDLS closed 3 years ago

LeeHudsonDLS commented 3 years ago

Hi, I'm not that familiar with the workings of StreamDevice but I have encountered an issue when trying to use this module at diamond light source. For years we've had a very old version so I recently pulled in the latest version. I see there is a commit where some data types were changed (6f4383c) in ChecksumConverter.cc, the part that seems to be causing the issue is the "inchar" variable which has changed from an unsigned int to a uint_fast8_t.

I see that the preprocessor substitutes uint_fast8_t for uint8_t if the target is vxWorks but it looks like on our version of vxWorks it should be 32bit (uint32_t). we are using vxWorks 5.5.1 running on a motorola MVME550.

If I change the preprocessor to use uint32_t and the sscanf calls to the old formatting it seems to work:

//sscanf(input(cursor+2*i), "%2" SCNx8, (int8_t *) &inchar); sscanf(input(cursor+2*i), "%2X", &inchar);

I'll try and get some more details tomorrow but I thought the cause of this may be more obvious to someone who knows the history.

dirk-zimoch commented 3 years ago

What do you mean with "on our version of vxWorks it should be 32bit"? Do you get a compiler warning? Please show. If possible also show the pre-processed code line.

I am using VxWorks 5.5.1 a lot (on MVME2300 and MVME5100) and have not seen a problem. inchar is a uint_fast8_t which on vxWorks is uint8_t, as you have noticed. That is "unsigned char". (Unless you have strangely modified your vxWorks). SCNx8 is "hhx" which makes the format string "%2hhx" which is the format for char.

LeeHudsonDLS commented 3 years ago

Sorry I have no experience of programming with vxWorks, i'm sure i'm wrong about the 32bit data types but the reason I came to that conclusion was because in order to get my checksum failures to go away I changed the following preprocessor line from: #define uint_fast8_t uint8_t

to: #define uint_fast8_t uint32_t

I also had to change the sscanf as detailed in my other comment.

I get no compiler warnings so I may be barking up the wrong tree but when I use a version of streamdevice after that commit where the data types are changed I find all my checksums fail:

2020/12/11 08:30:40.899212 ts1_17 LA02R-VA-IONP-01:ERRGET: Input "3D" does not match foormat "%0<sum8>" Older versions of streamdevice don't give these errors.

I did try to get to the bottom of this a year or so ago and i'm sure I had some more information from debugging about exactly why the checksum was failing, i'll dig this out and hopefully give you more information.

dirk-zimoch commented 3 years ago

Can you give me a sample input line (data+checksum)?

LeeHudsonDLS commented 3 years ago

Yes i'll do that now

LeeHudsonDLS commented 3 years ago

I've stripped down an IOC to a single record:

record(ai, "LA02R-VA-IONP-01:I") field(DTYP, "stream") field(INP, "@digitelMpcq.proto current(01,1) ts1_17")

The method in the protocol file is: out "~ \$1 0A \$2 %01<sum8>"; in "\$1 OK %*2x %g %*{AMPS|Amps} %0<sum8>";

The IOC writes the following to the device:

ascii: ~ 01 0A 1 83\r hex: 7e 20 30 31 20 30 41 20 31 20 38 33 0d

And the IOC receives the following:

ascii: 01 OK 00 5.9E-06 AMPS A0\r hex: 30 31 20 4f 4b 20 30 30 20 35 2e 39 45 2d 30 36 20 41 4d 50 53 20 41 30 0d

Which results in the error: 2020/12/11 09:09:19.268166 ts1_17 LA02R-VA-IONP-01:I: Input "A0" does not match format "%0"

dirk-zimoch commented 3 years ago

I'll have a look. The checksum A0 is correct according to my manual calculation. I will set up an IOC to see what I get...

LeeHudsonDLS commented 3 years ago

OK thanks

LeeHudsonDLS commented 3 years ago

For a direct comparison I modified the IOC to read a value that doesn't change (the current on this device is always changing) so i can compare linux to vxworks:

Protocol: out "~ \$1 0D \$2,00 %01<sum8>"; in "\$1 OK %*2x %*[^0-9]%d %0<sum8>";

vxWorks(read): 01 OK 00 02 3D\r 30 31 20 4f 4b 20 30 30 20 30 32 20 33 44 0d 2020/12/11 09:34:28.084794 ts1_17 LA02R-VA-IONP-01:ERRGET: Input "3D" does not match format "%0"

Linux (read): 01 OK 00 02 3D\r 30 31 20 4f 4b 20 30 30 20 30 32 20 33 44 0d

Not that this tells us much apart from linux doesn't see the issue. If you want me to put print statements in the code let me know and I can try that.

dirk-zimoch commented 3 years ago

I have tested on Linux with a fixed value, too. Everything fine. Setting up vxWorks now. I have to admit that I always run my tests on Linux only. It's much easier.

dirk-zimoch commented 3 years ago

Bug confirmed, I get the same problem. Let's see if vxWorks actually supports the standard "hh" field length modifier for char* args...

LeeHudsonDLS commented 3 years ago

Ah, i'm glad you can replicate it

dirk-zimoch commented 3 years ago

See here: https://stackoverflow.com/questions/63492012/sscanf-to-uint8-not-working-in-vxworks-platform

dirk-zimoch commented 3 years ago

From the vxWorks 5.5 Manual:

An optional h or l (el) indicating the size of the receiving object. The conversion specifiers d, i, and n should be preceded by h if the corresponding argument is a pointer to short int rather than a pointer to int, or by l if it is a pointer to long int. Similarly, the conversion specifiers o, u, and x shall be preceded by h if the corresponding argument is a pointer to unsigned short int rather than a pointer to unsigned int, or by l if it is a pointer to unsigned long int. Finally, the conversion specifiers e, f, and g shall be preceded by l if the corresponding argument is a pointer to double rather than a pointer to float. If an h or l appears with any other conversion specifier, the behavior is undefined.

It seems that vxWorks 5 does not know about hh for char. It probably reads is as h (for short) and converter n (undefined behavior) followed by a literal x.

Thanks for pointing this bug out. I will fix it immediately.

LeeHudsonDLS commented 3 years ago

OK great thanks!

dirk-zimoch commented 3 years ago

Fixed in 2.8.18.

LeeHudsonDLS commented 3 years ago

Wow that was quick! I'll pull that and try it now thanks

LeeHudsonDLS commented 3 years ago

That fixes it, thanks again