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

decoding of float and double is not working #18

Open
juwalter opened this issue Mar 26, 2023 · 1 comment
Open

decoding of float and double is not working #18

juwalter opened this issue Mar 26, 2023 · 1 comment
Assignees
Labels

Comments

@juwalter
Copy link

helle @lammertb

after some testing we noticed that libfins is seemingly broken for float and double

In libfins at

libfins/src/fins_01_04.c

Lines 185 to 217 in 91e9aa4

case FINS_DATA_TYPE_INT32 :
case FINS_DATA_TYPE_UINT32 :
case FINS_DATA_TYPE_BCD32 :
case FINS_DATA_TYPE_SBCD32_0 :
case FINS_DATA_TYPE_SBCD32_1 :
case FINS_DATA_TYPE_SBCD32_2 :
case FINS_DATA_TYPE_SBCD32_3 :
case FINS_DATA_TYPE_FLOAT :
if ( XX_finslib_decode_address( item[offset+a].address, & address ) ) return FINS_RETVAL_INVALID_READ_ADDRESS;
area_ptr = XX_finslib_search_area( sys, & address, 16, FI_MRD, false );
if ( area_ptr == NULL ) return FINS_RETVAL_INVALID_READ_AREA;
chunk_start = address.main_address;
chunk_start += area_ptr->low_addr >> 8;
chunk_start -= area_ptr->low_id;
fins_cmnd.body[bodylen++] = area_ptr->area;
fins_cmnd.body[bodylen++] = (chunk_start >> 8) & 0xff;
fins_cmnd.body[bodylen++] = (chunk_start ) & 0xff;
fins_cmnd.body[bodylen++] = 0x00;
chunk_start++;
fins_cmnd.body[bodylen++] = area_ptr->area;
fins_cmnd.body[bodylen++] = (chunk_start >> 8) & 0xff;
fins_cmnd.body[bodylen++] = (chunk_start ) & 0xff;
fins_cmnd.body[bodylen++] = 0x00;
recvlen += 6;
break;

case FINS_DATA_TYPE_INT32    :
case FINS_DATA_TYPE_UINT32   :
case FINS_DATA_TYPE_BCD32    :
case FINS_DATA_TYPE_SBCD32_0 :
case FINS_DATA_TYPE_SBCD32_1 :
case FINS_DATA_TYPE_SBCD32_2 :
case FINS_DATA_TYPE_SBCD32_3 :
case FINS_DATA_TYPE_FLOAT    :

	if ( XX_finslib_decode_address( item[offset+a].address, & address ) ) return FINS_RETVAL_INVALID_READ_ADDRESS;

	area_ptr = XX_finslib_search_area( sys, & address, 16, FI_MRD, false );
	if ( area_ptr == NULL ) return FINS_RETVAL_INVALID_READ_AREA;

	chunk_start  = address.main_address;
	chunk_start += area_ptr->low_addr >> 8;
	chunk_start -= area_ptr->low_id;

	fins_cmnd.body[bodylen++] = area_ptr->area;
	fins_cmnd.body[bodylen++] = (chunk_start >> 8) & 0xff;
	fins_cmnd.body[bodylen++] = (chunk_start     ) & 0xff;
	fins_cmnd.body[bodylen++] = 0x00;

	chunk_start++;

	fins_cmnd.body[bodylen++] = area_ptr->area;
	fins_cmnd.body[bodylen++] = (chunk_start >> 8) & 0xff;
	fins_cmnd.body[bodylen++] = (chunk_start     ) & 0xff;
	fins_cmnd.body[bodylen++] = 0x00;

	recvlen += 6;

	break;

the library treats FINS_DATA_TYPE_INT32, FINS_DATA_TYPE_UINT32, FINS_DATA_TYPE_BCD32, FINS_DATA_TYPE_SBCD32_0, FINS_DATA_TYPE_SBCD32_1, FINS_DATA_TYPE_SBCD32_2, FINS_DATA_TYPE_SBCD32_3, FINS_DATA_TYPE_FLOAT exactly the same when requesting data from the PLC.

However, during decoding:

(this seems correct for e.g. uint32)

case FINS_DATA_TYPE_UINT32 :

	item[offset+a].uint32   = fins_cmnd.body[bodylen+3];
	item[offset+a].uint32 <<= 8;
	item[offset+a].uint32  += fins_cmnd.body[bodylen+4];
	item[offset+a].uint32 <<= 8;
	item[offset+a].uint32  += fins_cmnd.body[bodylen+0];
	item[offset+a].uint32 <<= 8;
	item[offset+a].uint32  += fins_cmnd.body[bodylen+1];

	bodylen += 5;

	break;

but broken here:

libfins/src/fins_01_04.c

Lines 333 to 344 in 91e9aa4

case FINS_DATA_TYPE_FLOAT :
sfloat.val_raw[0] = fins_cmnd.body[0];
sfloat.val_raw[1] = fins_cmnd.body[1];
sfloat.val_raw[2] = fins_cmnd.body[3];
sfloat.val_raw[3] = fins_cmnd.body[4];
item[offset+a].sfloat = sfloat.val_float;
bodylen += 5;
break;

case FINS_DATA_TYPE_FLOAT :

	sfloat.val_raw[0] = fins_cmnd.body[0];
	sfloat.val_raw[1] = fins_cmnd.body[1];
	sfloat.val_raw[2] = fins_cmnd.body[3];
	sfloat.val_raw[3] = fins_cmnd.body[4];

	item[offset+a].sfloat = sfloat.val_float;

	bodylen += 5;

	break;

I cannot understand why using seemingly absolute offsets fins_cmnd.body[0]? this should also take bodylen into account or perhaps use pointer arithmetic to move fins_cmnd.body?

am I missing something?

@lammertb
Copy link
Owner

Seems like a bug TBH. The bytes should at least be aligned, not with a gap of one byte in between.

@lammertb lammertb added the bug label Jul 28, 2023
@lammertb lammertb self-assigned this Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants