| View previous topic :: View next topic |
| Author |
Message |
OldGuy
Joined: 03 Feb 2013
Posts: 27
Location: Seattle Area
|
| Signed number manipulation |
 Posted: Fri Oct 16, 2020 4:20 pm |
 |
|
Hello -
I am talking to an I2C device that returns a 12 bit 2s-complement number. It is read as the high byte in one location, and the low byte in another. The low byte has the data in the upper four bits, and the lower four bits are undefined.
I'm trying to put the two together into a signed int16, but the results don't seem correct. Can anyone check out my code to see what I'm doing incorrectly? This is easy enough for an unsigned int16, but the signed part makes it more complicated.
Following is my code, any help appreciated!
| Code: |
signed int8 StrikeL, StrikeH;
signed int16 StrikeData;
StrikeL = Read_Strike_Sensor_Register(0x0A);
StrikeH = Read_Strike_Sensor_Register(0x0B);
StrikeData = (signed int16) ((StrikeH << 8 ) | (StrikeL & 0xF0)) >> 4;
|
|
|
 |
temtronic
Joined: 01 Jul 2010
Posts: 9243
Location: Greensville,Ontario
|
|
| Posted: Fri Oct 16, 2020 8:15 pm |
|
|
You should post the I2C device, so we can see ...
but I'd probably read the two bytes in as unsigned bytes (lowerbyte, highnibl),
right shift the highnibl byte to get the high 4 bits ,
then use the make16() function . reading=make16(highnibl,lowerbyte)
this should give you the correct 12 bit data .
I don't know how to handle the 'signed' aspect of the data though, without seeing the datasheet of the device
with any 'math' calculation , I always break it down into easy to see 'steps'. While others can code complex functions this way, my old(er) brain doesn't work that way , anymore.... |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Sat Oct 17, 2020 2:07 am |
|
|
Do it the easy way. Do it like this:
| Code: |
StrikeData = make16(StrikeH, StrikeL); |
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19539
|
|
| Posted: Sat Oct 17, 2020 2:10 am |
|
|
Use a union (this is becoming a common answer!...).
| Code: |
union {
unsigned int8 bytes[2];
signed int16 whole;
} value;
value.bytes[0]=Read_Strike_Sensor_Register(0x0A);
value.bytes[1]=Read_Strike_Sensor_Register(0x0B);
value.whole/=16;
//Now gives the correctly signed result in single counts in
//value.whole
|
The issue with what you post is in two places. First, you are rotating the
high byte before you convert to an int16. This then results in the upper
bits being lost. Then whether you can rotate a signed value and
automatically 'sign extend' the result, varies with different compiler
versions. Most do direct rotation, not arithmetic rotation. Division,
guarantees sign extension, and will be done normally by rotation by
the compiler. |
|
|
OldGuy
Joined: 03 Feb 2013
Posts: 27
Location: Seattle Area
|
| Got it! |
| Posted: Sat Oct 17, 2020 10:12 am |
|
|
Thanks so much for the responses, much appreciated.
I've learned a few things - didn't know about the make16 function, nor how to deal with unions in a case like this.
You nailed the errors, temtronic. First error was shifting a byte and losing the data. Second error was shifting the merged bytes right, assuming that the sign bit got extended, which it didn't.
So this code fixed it:
| Code: |
signed int8 StrikeH;
int8 StrikeL;
signed int16 StrikeData;
StrikeL = Read_Strike_Sensor_Register(0x0A);
StrikeH = Read_Strike_Sensor_Register(0x0B);
StrikeData = (signed int16) (((signed int16) StrikeH << 8 ) | (StrikeL & 0xF0)) / 16;
|
Type-casting the StrikeH to an sint16 solved the lost data there, and changing the shift at the end to a divide solved the other.
Many thanks to you guys for helping me out. I was pulling my hair out.
- Brian |
|
|
OldGuy
Joined: 03 Feb 2013
Posts: 27
Location: Seattle Area
|
|
| Posted: Sat Oct 17, 2020 10:34 am |
|
|
| Oops, I credited the wrong person - Ttelmah was the solver. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19539
|
|
| Posted: Sat Oct 17, 2020 12:11 pm |
|
|
Can I suggest you try the union?.
Because you can put the values directly into this, it'll actually
be quite a bit faster than the make16. 
I think we are all just 'happy' when the solution is found. |
|
|
OldGuy
Joined: 03 Feb 2013
Posts: 27
Location: Seattle Area
|
|
| Posted: Sat Oct 17, 2020 12:43 pm |
|
|
| I switched to the union code, and it works well! Thanks for the education in unions. I can see how it is very efficient. |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Sat Oct 17, 2020 1:23 pm |
|
|
How is it faster than make16() ?
| Code: | .......... StrikeData = make16(StrikeH, StrikeL);
000C0: MOVFF StrikeH,StrikeData+1
000C4: MOVFF StrikeL,StrikeData |
It's only two lines of ASM code. |
|
|
OldGuy
Joined: 03 Feb 2013
Posts: 27
Location: Seattle Area
|
|
| Posted: Sat Oct 17, 2020 1:51 pm |
|
|
With the union approach, the values are read the first time into a merged word, so it eliminates the need to do a make16 or any moves entirely. It can be read out of the original location with no need to move or merge.
That's my understanding, anyway. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19539
|
|
| Posted: Sun Oct 18, 2020 12:44 am |
|
|
Spot on.
You are actually writing the values directly 'into' the location where they
are used. No need for the temporary variables, or any move. |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
