|
|
View previous topic :: View next topic |
Author |
Message |
septillion
Joined: 21 Jan 2010 Posts: 13
|
Different levels of indirection |
Posted: Wed Sep 28, 2016 4:50 am |
|
|
Hi all,
After some time not working with PICs I tried to make a new program for an older project. But I keep running into this error.
In my project I want to pass a struct to a function. No problem there but once I make an array of structs and try to loop over each I'm stuck with the error.
Of course my project is a bit bigger (but I wouldn't call it big) but I made a small example which gives the same error.
Code: |
#include <16F628A.h>
#fuses INTRC_IO,NOWDT,NOPROTECT,PUT, NOMCLR, BROWNOUT //internal clock, no watchdog, no code protection, power up timer, Mater Clear pin used for I/O, brownout protection
#use delay(clock=4000000) //set clock speed
typedef struct{
int one;
int two;
} _myStruct;
_myStruct myStructs[2];
void setHundred(_myStruct &in){
in.two = 100;
}
void main(){
myStructs[0].one = 2; //normal struct set, no problem
setHundred(myStructs[0]); //using the function on a array member, no error
//but this gives me the "Different levels of indirection"-error
for(byte i = 0; i < 2; i++){
setHundred(myStructs[i]); //this line
}
}
|
And to be complete, full error message:
Code: |
Error[105] structTest.c 26 : Different levels of indirection
1 Errors, 0 Warnings.
Build Failed.
|
Is it really not possible to loop over array items and pass them by reference to a function That would really break all possibility of scaling a program Or has anyone an alternative?
Timo |
|
|
jeremiah
Joined: 20 Jul 2010 Posts: 1358
|
|
Posted: Wed Sep 28, 2016 6:52 am |
|
|
Keep in mind that reference parameters are not a C construct but a C++ one and used as an extension to CCS C. They don't necessarily follow the rules of C++ since they are merely an extension.
This issue is that CCS uses reference parameters to pass *known at compile time* addresses through a function (and as a side effect it normally inlines that function as well).
myStructs[i] cannot have a known compile time address since i is not a compile time constant and changes at runtime.
That's why it thinks there are different levels of indirection (a bit of a loose definition IMO, but it is what it is).
To do what you want to do, you need to pass the parameter as a pointer:
Code: |
void setHundred(_myStruct *in){
in->two = 100;
}
|
and call it like:
Code: |
setHundred(&myStructs[i]);
|
|
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19552
|
|
Posted: Wed Sep 28, 2016 7:00 am |
|
|
It's perfectly possible.
You are having a problem because of C shortcuts, and getting misled about what they can do.
myStructs[n], is a structure of type _myStruct, not a pointer to such a structure. That is why you can access an element in such a reference directly. myStructs, is a shortcut to the address of the array.
This is only described in K&R, to work directly with the array name, not with a 'sub element' of the array. CCS happens to automatically treat a fixed index to such a sub element in the same way, but not with a variable, and it is honestly a little bit dangerous was to work.
So:
Code: |
for(byte i = 0; i < 2; i++){
setHundred(&myStructs[i]); //gives the address of structure 'i' in the array
|
Honestly, I'd only use the array name "on it's own" as the shortcut, and if wanting to pass the address of an element, always use the '&'. Safer, and avoids differences between C versions.
The other way is to use the offset in a different way:
Code: |
for(byte i = 0; i < 2; i++){
setHundred(myStructs+i); //gives the address of structure 'i'
|
This is because 'myStructs' is the address of the first element, and if you add to such a pointer, you automatically increment in 'element sized' lumps, so 'myStructs_+i', is defined as the address of element 'i' in the array.
The fact you are using reference parameters in the target function, does not really affect things. It still expects to receive an address, it is just that it then 'works back' and works on the source variable, not the copy in the function.
For a structure, why not just use the reference access mode (using -> instead of '.' to access the sub variable)?. I see Temtronic has said this as well. It is a much safer way of working.... |
|
|
septillion
Joined: 21 Jan 2010 Posts: 13
|
|
Posted: Wed Sep 28, 2016 8:21 am |
|
|
First of all, thanks guys!
I'm indeed more of a C++ guy I started with PIC but moved to AVR (or ARM now) because of the open tool chains. But this is still an old project I want to revise which means, reprogramming the PIC.
But you say this can't be done by reference in CCS C? I did it by reference instead of pointer because I think it just looks cleaner to call something like myFunction(instance) then to have to add the address operator.
In the past I have used this code (button debounce with passing by reference) multiple times without a problem but always on a single button. And I could make multiple structs insteads of an array of structs but that would make looping over them harder (aka, harder to scale).
But if by pointer will work fine I will just use that.
And about the operator, what is the difference between '.' and '->' as an operator? |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19552
|
|
Posted: Wed Sep 28, 2016 8:37 am |
|
|
Three different versions:
By pointer.
By reference pointer.
By reference.
By reference, is not a C ability. CCS offers it, but with limited abilities. Has to be directly 'to' a variable.
By pointer - standard.
By reference pointer, is for things like structures. On this you can pass a pointer 'to' a function, but then use it inside the function without having to de-reference it, to operate directly on the original variable. It is to do exactly what you are trying to do. So:
Code: |
#include <16F628A.h>
#fuses INTRC_IO,NOWDT,NOPROTECT,PUT, NOMCLR, BROWNOUT //internal clock, no watchdog, no code protection, power up timer, Mater Clear pin used for I/O, brownout protection
#use delay(clock=4000000) //set clock speed
typedef struct{
int one;
int two;
} _myStruct;
_myStruct myStructs[2];
void setHundred(_myStruct *in){
in->two = 100; //note this change
}
void main(void)
{
myStructs[0].one = 2; //
setHundred(myStructs); //using the function on first array member
//Or
setHundred(&myStructs[1]); //second
//Or
setHundred(myStructs+2); //and third
for(byte i = 0; i < 2; i++){
setHundred(&myStructs[i]); //use second method for now
}
}
|
|
|
|
jeremiah
Joined: 20 Jul 2010 Posts: 1358
|
|
Posted: Wed Sep 28, 2016 9:00 am |
|
|
septillion wrote: |
And about the operator, what is the difference between '.' and '->' as an operator? |
'.' is element/member selector and is used directly on structs:
Code: |
_myStruct struct1;
struct1.one = 1;
|
'->' is a dereference pointer and select element/membor. This is used with pointers to structs:
Code: |
_myStruct * struct1_ptr;
struct1_ptr = &struct1; //save the address of struct1 in the pointer
struct1_ptr->one = 1; //does the same thing as struct1.one = 1;
|
alternate syntax to the '->' is:
Code: |
(*struct1_ptr).one = 1;
|
but I guess they decided that was too messy and added the arrow operator instead. |
|
|
septillion
Joined: 21 Jan 2010 Posts: 13
|
|
Posted: Wed Sep 28, 2016 10:24 am |
|
|
Thanks for explaining guys!
Alright, I tried to change my small library to use pass by pointers instead of pass by reference. So I change all the references to pointer (from &b to *b) and all the member operators to dereference member operators (brom b. to b->). And in my code I added the address of operator in front of struc variable names. And tada, it broke it
So my library to debounce buttons, and yes, it's heavily inspired by Arduino
With pass by reference.
Code: |
#define millis() get_ticks()
typedef struct{
int pin;
long lastMillis;
int1 debouncedState;
int1 unstableState;
int1 changed;
} _button;
void buttonAttach(_button &b, int pin);
int1 buttonUpdate(_button &b);
int1 buttonRead(_button &b);
int1 buttonRose(_button &b);
int1 buttonFell(_button &b);
int1 buttonChanged(_button &b);
void buttonAttach(_button &b, int pin){
b.pin = pin;
if(input(b.pin)){
b.debouncedState = 1;
b.unstableState = 1;
}
else{
b.debouncedState = 0;
b.unstableState = 0;
}
b.changed = 0;
b.lastMillis = millis();
}
int1 buttonUpdate(_button &b){
// Read the state of the switch in a temporary variable
int1 currentState = input(b.pin);
b.changed = 0; //and clear changed
// If the reading is different from last reading, reset the debounce counter
if ( currentState != b.unstableState) {
b.lastMillis = millis();
b.unstableState = currentState; //flip unstable state
}
//Reading is the same, long enough?
else if( millis() - b.lastMillis >= 10 ){
// We have passed the threshold time, so the input is now stable
// If it is different from last state, set the STATE_CHANGED flag
if (b.debouncedState != currentState) {
b.lastMillis = millis();
b.debouncedState = currentState; //flip debounced state
b.changed = 1; //state state changed
}
}
return b.changed;
}
int1 buttonRead(_button &b){
return b.debouncedState; //return debounced state
}
int1 buttonRose(_button &b){
//debounced state high and state changed
return (b.debouncedState && b.changed);
}
int1 buttonFell(_button &b){
//debounced state low and state changed
return !b.debouncedState && b.changed;
}
//Return true if state changed
int1 buttonChanged(_button &b){
return b.changed;
}
|
Test code:
Code: |
#include <16F648A.h>
#fuses INTRC_IO,NOWDT,NOPROTECT,PUT, NOMCLR, BROWNOUT //internal clock, no watchdog, no code protection, power up timer, Mater Clear pin used for I/O, brownout protection
#use delay(clock=4000000) //set clock speed
//Setup timer for get_ticks() / millis()
#USE TIMER(TIMER=1, TICK=1ms, BITS=16, ISR)
#include <debounce.c>
#define millis() get_ticks()
//const int8 BellButtonPins[] = {PIN_A2, PIN_A3};
const int8 ButtonPin = PIN_A2;
const int8 OutputPin = PIN_A0;
const int8 AnotherOutputPin = PIN_A1;
_button myButton;
void main(){
enable_interrupts(GLOBAL);
buttonAttach(myButton, ButtonPin);
while(true){
//output_bit(PIN_A0, input(PIN_A2));
buttonUpdate(myButton);
output_bit(OutputPin, !buttonRead(myButton));
//calling it without debounce
output_bit(AnotherOutputPin, !input(ButtonPin));
}
} |
Works like a charm, leds on both outputs turn on. One via debounce.
So after changing it:
library
Code: |
#define millis() get_ticks()
typedef struct{
int pin;
long lastMillis;
int1 debouncedState;
int1 unstableState;
int1 changed;
} _button;
void buttonAttach(_button *b, int pin);
int1 buttonUpdate(_button *b);
int1 buttonRead(_button *b);
int1 buttonRose(_button *b);
int1 buttonFell(_button *b);
int1 buttonChanged(_button *b);
void buttonAttach(_button *b, int pin){
b->pin = pin;
if(input(b->pin)){
b->debouncedState = 1;
b->unstableState = 1;
}
else{
b->debouncedState = 0;
b->unstableState = 0;
}
b->changed = 0;
b->lastMillis = millis();
}
int1 buttonUpdate(_button *b){
// Read the state of the switch in a temporary variable
int1 currentState = input(b->pin);
b->changed = 0; //and clear changed
// If the reading is different from last reading, reset the debounce counter
if ( currentState != b->unstableState) {
b->lastMillis = millis();
b->unstableState = currentState; //flip unstable state
}
//Reading is the same, long enough?
else if( millis() - b->lastMillis >= 10 ){
// We have passed the threshold time, so the input is now stable
// If it is different from last state, set the STATE_CHANGED flag
if (b->debouncedState != currentState) {
b->lastMillis = millis();
b->debouncedState = currentState; //flip debounced state
b->changed = 1; //state state changed
}
}
return b->changed;
}
int1 buttonRead(_button *b){
return b->debouncedState; //return debounced state
}
int1 buttonRose(_button *b){
//debounced state high and state changed
return (b->debouncedState && b->changed);
}
int1 buttonFell(_button *b){
//debounced state low and state changed
return !b->debouncedState && b->changed;
}
//Return true if state changed
int1 buttonChanged(_button *b){
return b->changed;
}
|
Test code:
Code: |
#include <16F648A.h>
#fuses INTRC_IO,NOWDT,NOPROTECT,PUT, NOMCLR, BROWNOUT //internal clock, no watchdog, no code protection, power up timer, Mater Clear pin used for I/O, brownout protection
#use delay(clock=4000000) //set clock speed
//Setup timer for get_ticks() / millis()
#USE TIMER(TIMER=1, TICK=1ms, BITS=16, ISR)
#include <debounce.c>
#define millis() get_ticks()
//const int8 BellButtonPins[] = {PIN_A2, PIN_A3};
const int8 ButtonPin = PIN_A2;
const int8 OutputPin = PIN_A0;
const int8 AnotherOutputPin = PIN_A1;
_button myButton;
void main(){
enable_interrupts(GLOBAL);
buttonAttach(&myButton, ButtonPin);
while(true){
//output_bit(PIN_A0, input(PIN_A2));
buttonUpdate(&myButton);
output_bit(OutputPin, !buttonRead(&myButton));
//calling it without debounce
output_bit(AnotherOutputPin, !input(ButtonPin));
}
}
|
I would think it should still do the same but only the direct controlled output turns on :/ What am I missing? |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19552
|
|
Posted: Wed Sep 28, 2016 12:37 pm |
|
|
I didn't look very far, but saw one glaring thing straight away. A pin number in a PIC, is an int16, not an int8. Remember 'int' in CCS is an int8. This will stop it working. |
|
|
septillion
Joined: 21 Jan 2010 Posts: 13
|
|
Posted: Wed Sep 28, 2016 12:49 pm |
|
|
Why is it a int16? It's just defined as a macro. And on a PIC16F648A/628A it's a number between 40 and 55. This might be different for other PICs but at least for the 16F648A I don't see a problem. We can assign that to an int8 just fine.
And just to check I'm not to stubborn I changed the variable for the pin in the struct, attach-function and in the test code to int16 but that didn't change a thing. |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19552
|
|
Posted: Wed Sep 28, 2016 1:18 pm |
|
|
Fair enough. I haven't used a chip where it is an int8 for ages.
It's actually the register number*8, plus the bit number.
It has to be an int16, if any pin on the chip requires an int16.
That does look as if it should work.
Why do you waste storage declaring the pin numbers?. Just use #defines for these. |
|
|
septillion
Joined: 21 Jan 2010 Posts: 13
|
|
Posted: Wed Sep 28, 2016 1:33 pm |
|
|
Like I said, old project, good old 16F628A (/648A)
And yeah, I thought it would be something like that. Didn't dive into it So yeah, for compatibility it should be int16 but for the 16F648A int8 isn't the problem.
And I'm "wasting memory" because this makes it easy to put pin numbers into an array and just loop over them. Great for scaling things.
But because they are const they should not waste memory because they are located in ROM. At least, that seems to be the default if I understand the help of #DEVICE correctly...
And last but not least, the passed by reference version works fine, the passed by pointer does not |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19552
|
|
Posted: Wed Sep 28, 2016 3:05 pm |
|
|
Making them into const, wastes a surprising amount of memory.....
Problem is there has to be code to read them. If you use a #define, the number is written directly inside the code at compile time. A variable, still has to be read.
Critical question now comes. What compiler version number are you using?.
Problems with things like pointers (and references etc..), are typical for some of the older compiler versions. What version are you on?. |
|
|
septillion
Joined: 21 Jan 2010 Posts: 13
|
|
Posted: Wed Sep 28, 2016 4:08 pm |
|
|
Mm, true, but the fact it's scaleable is worth it
I'm using 5.008, on old Uni license.... It's a bug there? :/ |
|
|
jeremiah
Joined: 20 Jul 2010 Posts: 1358
|
|
Posted: Thu Sep 29, 2016 6:33 am |
|
|
Can't tell if it is a bug or not, but it appears to be with your use of int1's. If I change all the int1 variables in the struct and in buttonUpdate to unsigned int8's I get it to work.
I also changed lastMillis to be unsigned (it is an unsigned value, not a signed value, so the variable should match).
Code: |
#define millis() get_ticks()
typedef struct{
int16 pin;
unsigned int16 lastMillis;
unsigned int8 debouncedState;
unsigned int8 unstableState;
unsigned int8 changed;
} _button;
void buttonAttach(_button *b, int16 pin);
int1 buttonUpdate(_button *b);
int1 buttonRead(_button *b);
int1 buttonRose(_button *b);
int1 buttonFell(_button *b);
int1 buttonChanged(_button *b);
void buttonAttach(_button *b, int16 pin){
b->pin = pin;
if(input(b->pin)){
b->debouncedState = 1;
b->unstableState = 1;
}
else{
b->debouncedState = 0;
b->unstableState = 0;
}
b->changed = 0;
b->lastMillis = millis();
}
int1 buttonUpdate(_button *b){
// Read the state of the switch in a temporary variable
unsigned int8 currentState = input(b->pin);
b->changed = 0; //and clear changed
// If the reading is different from last reading, reset the debounce counter
if ( currentState != b->unstableState) {
b->lastMillis = millis();
b->unstableState = currentState; //flip unstable state
}
//Reading is the same, long enough?
else if( millis() - b->lastMillis >= 10 ){
// We have passed the threshold time, so the input is now stable
// If it is different from last state, set the STATE_CHANGED flag
if (b->debouncedState != currentState) {
b->lastMillis = millis();
b->debouncedState = currentState; //flip debounced state
b->changed = 1; //state state changed
}
}
return b->changed;
}
int1 buttonRead(_button *b){
return b->debouncedState; //return debounced state
}
int1 buttonRose(_button *b){
//debounced state high and state changed
return (b->debouncedState && b->changed);
}
int1 buttonFell(_button *b){
//debounced state low and state changed
return !b->debouncedState && b->changed;
}
//Return true if state changed
int1 buttonChanged(_button *b){
return b->changed;
}
|
I am compiling with 5.061 and using a PIC24FJ64GA004 since I don't have any 8 bit chips lying around.
You might send an email to CCS support with both files and see what they say about it. |
|
|
septillion
Joined: 21 Jan 2010 Posts: 13
|
|
Posted: Fri Sep 30, 2016 7:29 am |
|
|
Thanks for testing! I will give it a try once I'm back home. Did not bring my PicKit 2 with me.
I first wanted to say a int1 is just defined as a int8 but then I saw that's only the case if the compiler isn't PCM. So int1 is a build in type.
I did not specify the unsigned type because the CCS Help says "Unsigned: Data is always positive. This is the default data type if not specified.". But I agree it's nicer to specify it specifically.
And because how a pin number is build up, isn't that a unsigned as well? (Not that it matters because of the unsigned as default type.)
I will also send an email. Because it's indeed a bit strange it does work with a int8. For now I'll just use a int8 to do it, project is small and I can handle the extra memory. But I think I'll change the library to use one byte for all the states (different bits in the same byte) to same some.
But thanks again or helping! |
|
|
|
|
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
|