|
|
View previous topic :: View next topic |
Author |
Message |
aodj
Joined: 20 Dec 2006 Posts: 40 Location: Reading, UK
|
Incorrect addition |
Posted: Mon May 14, 2007 8:38 am |
|
|
I'm currently debugging some relatively simple and straight forward code, however I'm coming up against a strange problem.
Code: | 686: vCPrimary++;
8A4 3002 MOVLW 0x2
8A5 07E3 ADDWF 0x63, F
8A6 1803 BTFSC 0x3, 0
8A7 0AE4 INCF 0x64, F
687: vCPrimary += 1;
8A8 3002 MOVLW 0x2
8A9 07E3 ADDWF 0x63, F
8AA 1803 BTFSC 0x3, 0
8AB 0AE4 INCF 0x64, F |
For some reason, when I run either of these lines, the value held in vCPrimary, increases by 2, as can be seen with the MOVLW 0x2 (I think, my assembler skills are pretty nonexistent). I only need to increase vCPrimary by 1, but both are present above for the sake of evaluation.
Does anyone know a reason why this would be? vCPrimary is initialised as signed int16 **vCPrimary. |
|
|
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
|
aodj
Joined: 20 Dec 2006 Posts: 40 Location: Reading, UK
|
|
Posted: Mon May 14, 2007 9:09 am |
|
|
My bad, I should have stated, I'm running 4.016. |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Tue May 15, 2007 6:34 pm |
|
|
Quote: |
vCPrimary++;
vCPrimary += 1;
For some reason, when I run either of these lines, the value held
in vCPrimary, increases by 2, as can be seen with the MOVLW 0x2 . |
If a pointer to an 'int16' is incremented by 1, this tells the compiler
that you want to point to the next 16-bit word. Since the data memory
in an 18F PIC is addressed as bytes, the compiler is correct when it
adds 2 to the address. It will point to the next 'int16' value. |
|
|
aodj
Joined: 20 Dec 2006 Posts: 40 Location: Reading, UK
|
|
Posted: Wed May 16, 2007 1:38 am |
|
|
Do you have any idea how I would go about incrementing by just one? I've attempted to alter the statement and the pointers around it, but my grasp of pointers is somewhat lacking. |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Wed May 16, 2007 1:55 am |
|
|
Cast it to a char pointer. But watch your operator precedence, because
the '++' operator is done before the casting. You need to use parens
in that case. But you don't need to use them with the '+ 1' method.
Code: |
((char *)vCPrimary)++;
(char *)vCPrimary += 1;
|
Maybe you should tell us why you want to increment a word pointer by
one byte. We might have a better way to solve your problem. |
|
|
aodj
Joined: 20 Dec 2006 Posts: 40 Location: Reading, UK
|
|
Posted: Wed May 16, 2007 4:35 am |
|
|
The application is thus:
I'm writing a subroutine which handles increments in a range for a temperature sensing and handling unit.
The subroutine works for both fahrenheit and celsius scales, between a range that is defined for each seperate call; for some the range is -10C to +10C (-18F to +18F) as a range, others are a direct celsius/fahrenheit conversion.
IE, sometimes I call a second, nested subroutine, to convert for ranges, other times i call for a straight conversion (think a 1C increase is an increase of 1.8F, and 25C is 77F)
Each time I call this, I have to pass the variables, the memory locations, boundary points, conversion type to the initial subroutine.
Code: | void fIncrement(signed int16 *vPrimary, signed int16 *vFPrimary, signed int16 *vCPrimary, signed int16 vFLow, signed int16 vCLow, signed int16 vFHigh, signed int16 vCHigh, int1 bConvertType, int8 **vMFPrimary, int8 **vMCPrimary)
{
//
// Variable handler
//
if(bScale == 1) // Fahrenheit
{
vPrimary = &vFPrimary;
}
else // Celsius
{
vPrimary = &vCPrimary;
}
fsprintf(*vPrimary);
//
// Button handler
//
bButton = 1; // Allows the routine to detect button pushes. Helps minimise errornous inputs.
if(bB4 == 1) // Button 4
{
if((vFPrimary > vFLow) && (bScale == 1)) // Fahrenheit
{
vCPrimary = fFCConvert(--(*vPrimary), bConvertType);
}
if((vCPrimary > vCLow) && (bScale == 0)) // Celsius
{
vFPrimary = fCFConvert(--(*vPrimary), bConvertType);
}
bB4 = 0;
}
if(bB5 == 1) // Button 5
{
if((vFPrimary < vFHigh) && (bScale == 1)) // Fahrenheit
{
vCPrimary = fFCConvert(++(*vPrimary), bConvertType);
}
if((vCPrimary < vCHigh) && (bScale == 0)) // Celsius
{
vFPrimary = fCFConvert(++(*vPrimary), bConvertType);
}
bB5 = 0;
}
if(bB6 == 1) // Button 6
{
if(bScale == 1) // Fahrenheit, 16 bit
{
write_ee16(vMFPrimary, *vPrimary); // location, data
write_ee16(vMCPrimary, vCPrimary);
}
else if(bScale == 0) // Celsius, 16 bit
{
write_ee16(vMCPrimary, *vPrimary);
write_ee16(vMFPrimary, vFPrimary);
}
bButton = 0;
bB6 = 0;
}
if(bB7 == 1) // Button 7
{
vMenu++; // Used in a case statment to specify which menu is currently in effect
vTimeout = 0; // Menu timeout value
bPlayOnce = 0; // Display text once flag
bButton = 0; //
bB7 = 0;
}
} |
The different routines I call are:
fsprintf(), a wrapper for the sprintf() function.
fFCConvert(), a fahrenheit to celsius conversion routine.
fCFConvert(), a celsius to fahrenheit conversion routine.
write_ee16(), a wrapper for write_ee8() which is a wrapper for write_eeprom().
This routine would be called, for example, with:
Code: | fIncrement(vSetpoint, vFSetpoint, vCSetpoint, -58, -50, 212, 100, 1, mFSetpoint, mCSetpoint); // -50C = -58F, 100C = 212F |
// This defines the range of operation at -50C/-58F to 100C/212F and allows you to scroll through any value within that range, and then save it to EEPROM as specified by the mCSetpoint/mFSetpoint values.
-or-
Code: | fIncrement(vDifferential1, vFDifferential1, vCDifferential1, 0, 0, 18, 10, 0, mFDifferential1, mCDifferential1); |
// This sets the ranges at 0C/0F to 10C/18F, which is a range conversion, not a direct equivalent conversion as specified by the '0' before mFDifferential1.
Now obviously I realise I'm passing alot of varaiables to the subroutine, and that my pointer handling may not be perfect. I did manage to get the pointer to increment the value of the data it's looking at, through, as you suggested, using parentheses to sort out precedents. Unfortanately, I have no way to pass the data back to the original locations. |
|
|
Ttelmah Guest
|
|
Posted: Wed May 16, 2007 7:42 am |
|
|
First thing to remember. You can explicitly cast, or you can convert types on the fly. If (for instance), you have a routine to write an int16 to the EEPROM, and define it like this:
write_int16(int8 * RAM_address, int8 address_in_eeprom)
Then call this function with the address of the 16bit value, the 'int16 *' type of the address passed to the routine, is automatically converted to an 'int8 *' in the routine, allowing you to access the int16 (or multiple int16 values), 'byte by byte', witout needing any explicit casting.
This ability, is both a strength, and a weakness of C. It is strong, in that it allows you to ignore many of the problems of dealing with different data types, by performing such 'on the fly' conversions, but is weak (dangerous), since the implications of such 'mixed size' handling, can make errors hard to trace.
Now there would seem to be nothing very difficult in what you want to do. If you want to pass a value, and modify it, then pass the address, and modify the addressed value in the subroutine. The conversion to allow you to work with 'bytes' for working in the EEPROM, can be handled by automatic conversion as shown above, or by explicit casting. You can also 'cheat', for instance:
Code: |
union access {
float value;
int8 b[4]
};
//Then a routine like:
void float_to_eeprom(union access * val, int8 number_to_send, int8 start_address) {
int8 ctr;
float * temp_ptr;
//only used for the demo return
temp_ptr=val;
while (number_to_send--) {
//repeat for the specified number of 'float' values
for (ctr=0;ctr<4>b[ctr]);
}
//demonstrate _changing_ a source value
temp_ptr[9]=9.00;
//Similar, but with an explicit cast:
((union access *)temp_ptr)->value=1.00;
}
//Called with (say):
int32 float_values[10];
//set some values up here
float_to_eeprom(float_values,10,0);
|
This will pass the address of the 'float' values, to the routine, where it'll be implicitly converted to the address of 'union access' values.
Then this address is used to access the bytes (using the byte array declared in the union), incrementing the pointer forwards by the size of the union on each loop (so 40 bytes will be sent to the eeprom). Then the last floating point value in the array is accessed, and changed to a '9.00', to show how this can also deal with returning values, by using another implicit cast.
The same operation, on the first value, is then done using an explicit cast, this time changing it to '1.00'.
Effectively, this maintains access to both the 'byte' values, and also the 'float' values, by using this explict conversion during the call...
Best Wishes |
|
|
aodj
Joined: 20 Dec 2006 Posts: 40 Location: Reading, UK
|
|
Posted: Thu May 17, 2007 1:34 am |
|
|
Whilst the code you posted allows me more flexibility when it coems to handling different data types, my grasp of C isn't enough for me to totally understand what you wrote.
So it may be more versatile and efficient that anything I was able to write, but I would be unable to maintain it once it was in use, with my current C knowledge.
Thanks though |
|
|
Ttelmah Guest
|
|
Posted: Thu May 17, 2007 2:31 am |
|
|
The key point I was making, was to make things simpler!.
Don't get involved in casting pointers. Instead, if you have a routine that needs to deal with bytes (the EEPROM save/restore), make this implicitly convert the pointer type. This way, your main routines (which deal with floats), can be handed float pointers, and work with these, while the rotuines that deal with the other types, deal _locally_ with these same pointers, automatically treated as the type needed, without casting being needed..
This is far the safest way of handling such conversions.
I then went 'on', showing how a single routine, could change the pointer type if needed, but this is normally unecessary.
Best Wishes |
|
|
|
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|