Why does my debug routine fix the program?

Thread Starter

AlbertHall

Joined Jun 4, 2014
12,346
MPLABX V6.0, XC8 v2.40

At the start of the program:
Code:
enum DisplayText_t{OFF_T, MAINS_T, REMAINING_T, RAW_T, LIMIT_T, LIMIT_PLUS_T, LIMIT_MINUS_T};
const char *Text[] = {"      ", "Mains ", "Spare ", "Raw   ", "Limit ", "Limit+", "Limit-"};
Where the value of 'Text' is used:
Code:
        SSD1306_GotoXY(1,3);
        SSD1306_puts(Text[NewText], 2);
        LastText = NewText;
The problem is that 'Text[NewText]' is nul and so the display is not written. This is regardless of the optimisation setting.
I added the code below to check the values in the array:
Code:
    uint8_t Test;
    for(uint8_t i = 0; i < 7; i++)
    {
        Test = Text[0][i];
        if(Test == 0x20)
            NOP();
    }
The array contents checked out OK, but with this code in place the display writing works correctly.

If necessary I can leave the extra code in place, but I cannot see why this happens.
 

Ya’akov

Joined Jan 27, 2019
9,136
I am not expert in C at all and I expect you will get a clear answer from someone much more knowledgeable, but I wonder if it would work with static, like static const char *Text[] = {" ", "Mains ", "Spare ", "Raw ", "Limit ", "Limit+", "Limit-"};.
 

dcbingaman

Joined Jun 30, 2021
1,065
Try changing this from:

const char *Text[] = {" ", "Mains ", "Spare ", "Raw ", "Limit ", "Limit+", "Limit-"};

To:

static const char const * const Text[] = { " ", "Mains ", "Spare ", "Raw ", "Limit ", "Limit+", "Limit-" };

I wrote a small application in C that seems to be working correctly even with the original definition you gave:

1662302932408.png
 

dcbingaman

Joined Jun 30, 2021
1,065
You may want to add this as well, that will allow the array size to be determined at run time so you don't have to do it manually:

1662303227433.png
 

dcbingaman

Joined Jun 30, 2021
1,065
This prints out the strings as expected. I don't normally use puts() but it seems to work without issue. This is Visual Studio C compiler:

1662303691845.png
MPLABX V6.0, XC8 v2.40

At the start of the program:
Code:
enum DisplayText_t{OFF_T, MAINS_T, REMAINING_T, RAW_T, LIMIT_T, LIMIT_PLUS_T, LIMIT_MINUS_T};
const char *Text[] = {"      ", "Mains ", "Spare ", "Raw   ", "Limit ", "Limit+", "Limit-"};
Where the value of 'Text' is used:
Code:
        SSD1306_GotoXY(1,3);
        SSD1306_puts(Text[NewText], 2);
        LastText = NewText;
The problem is that 'Text[NewText]' is nul and so the display is not written. This is regardless of the optimisation setting.
I added the code below to check the values in the array:
Code:
    uint8_t Test;
    for(uint8_t i = 0; i < 7; i++)
    {
        Test = Text[0][i];
        if(Test == 0x20)
            NOP();
    }
The array contents checked out OK, but with this code in place the display writing works correctly.

If necessary I can leave the extra code in place, but I cannot see why this happens.
If what I recommended does not work you could also try this:

1662304373151.png

By placing the array of pointers into a function with static qualifiers, this will get initialized the first time the function is called per C standards. I have a feeling the compiler is not initializing the array until required and it does not feel it was required in the original situation. Also by making it all constant, the compiler should now put the array into ROM instead of RAM.
 

Thread Starter

AlbertHall

Joined Jun 4, 2014
12,346
XC8 complained about the third 'const' in the array definition.
With that removed it complained that there was no type specifier for TextLen - I added 'int'.
With those modifications the code works and solves the problem - except then the display blinks sometimes which I will have to investigate.

It does seem odd to need such a big hammer to hit the compiler over the head and tell it not to be so stupid?
 
Last edited:

Thread Starter

AlbertHall

Joined Jun 4, 2014
12,346
Try changing this from:

const char *Text[] = {" ", "Mains ", "Spare ", "Raw ", "Limit ", "Limit+", "Limit-"};

To:

static const char const * const Text[] = { " ", "Mains ", "Spare ", "Raw ", "Limit ", "Limit+", "Limit-" };
Adding the 'static' made no difference.
 

Thread Starter

AlbertHall

Joined Jun 4, 2014
12,346
I am not expert in C at all and I expect you will get a clear answer from someone much more knowledgeable, but I wonder if it would work with static, like static const char *Text[] = {" ", "Mains ", "Spare ", "Raw ", "Limit ", "Limit+", "Limit-"};.
Nope, the 'static' makes no difference.
 

dcbingaman

Joined Jun 30, 2021
1,065
XC8 complained about the third 'const' in the array definition.
With that removed it complained that there was no type specifier for TextLen - I added 'int'.
With those modifications the code works and solves the problem - except then the display blinks sometimes which I will have to investigate.

It does seem odd to need such a big hammer to hit the compiler over the head and tell it not to be so stupid?
Curious: was it the const specifier that fixed the issue or was it placing it into a function that fixed the issue?

FYI: I generally use the PIC32 series with the XC32 compiler for MPLab. I have not ran across this issue using that compiler.
 

Thread Starter

AlbertHall

Joined Jun 4, 2014
12,346
Curious: was it the const specifier that fixed the issue or was it placing it into a function that fixed the issue?

FYI: I generally use the PIC32 series with the XC32 compiler for MPLab. I have not ran across this issue using that compiler.
Putting it into a function without that third 'const' gets the array initialised.
 

dcbingaman

Joined Jun 30, 2021
1,065
The const on the right side prohibits the pointer from being changed. On my compiler:
static char * const Text[] = {

Makes this illegal:

char x='C';
Text[0]=&x;

But this is legal:

Text[0][0]='C';

I think these three are the same and result in the contents (the strings themselves) being const:
1 const char *Text[]=...
2 char const *Text[]=...
3 const char const *Text[]=...

In the last case the const before and after char is redundant, but my compiler accepts it.

This is now illegal
Text[0][0]='C';

But this is legal:
Text[0]=&x;

Is your compiler complaining about the const before and after char? That might make sense being that in that case the second const is redundant.

So are you doing this which should be legal with no redundancy:

static const char * const Text[]=...

That is legal because there is no redundancy. You should be able to specify it this way as well which is legal:

static char const * const Text[]=...

This is redundant but my compiler still accepts it (you only need one const before or after char to specify the contents are const:

static const char const * const Text[]=...

In my opinion the compiler should not accept this because the const qualifier is redundant, but my compiler has no issues with that.

BTW: I forgot to add int to this and my compiler accepted it:

static const TextLen = sizeof(Text)/sizeof(char*);

IMHO it should not because the type is not specified. I think the compiler should force you to specify the type as that is more correct, like yours did for the XC8 compiler.
 

dcbingaman

Joined Jun 30, 2021
1,065
XC8 complained about the third 'const' in the array definition.
With that removed it complained that there was no type specifier for TextLen - I added 'int'.
With those modifications the code works and solves the problem - except then the display blinks sometimes which I will have to investigate.

It does seem odd to need such a big hammer to hit the compiler over the head and tell it not to be so stupid?
I agree that needing to place this in a function should not be required. Not sure why the compiler is acting the way it is with your code. The compiler is acting 'stupid' as you say.
I feel for those people that have to write C compilers (or any compiler for that matter). Because it has to handle a lot of situations and types. It is hard to write compilers properly because how do you cover all possible methods of handling things? There is so much variation from one Programmer to the next.
 
Last edited:

upand_at_them

Joined May 15, 2010
940
MPLABX V6.0, XC8 v2.40

At the start of the program:
Code:
enum DisplayText_t{OFF_T, MAINS_T, REMAINING_T, RAW_T, LIMIT_T, LIMIT_PLUS_T, LIMIT_MINUS_T};
const char *Text[] = {"      ", "Mains ", "Spare ", "Raw   ", "Limit ", "Limit+", "Limit-"};
Where the value of 'Text' is used:
Code:
        SSD1306_GotoXY(1,3);
        SSD1306_puts(Text[NewText], 2);
        LastText = NewText;
The problem is that 'Text[NewText]' is nul and so the display is not written. This is regardless of the optimisation setting.
You don't how the declaration of NewText or its value being set. By the way, adding "static" in front of the declaration for *Text[] only limits its scope to that file.
 

WBahn

Joined Mar 31, 2012
30,045
MPLABX V6.0, XC8 v2.40

At the start of the program:
Code:
enum DisplayText_t{OFF_T, MAINS_T, REMAINING_T, RAW_T, LIMIT_T, LIMIT_PLUS_T, LIMIT_MINUS_T};
const char *Text[] = {"      ", "Mains ", "Spare ", "Raw   ", "Limit ", "Limit+", "Limit-"};
Where the value of 'Text' is used:
Code:
        SSD1306_GotoXY(1,3);
        SSD1306_puts(Text[NewText], 2);
        LastText = NewText;
The problem is that 'Text[NewText]' is nul and so the display is not written. This is regardless of the optimisation setting.
I added the code below to check the values in the array:
Code:
    uint8_t Test;
    for(uint8_t i = 0; i < 7; i++)
    {
        Test = Text[0][i];
        if(Test == 0x20)
            NOP();
    }
The array contents checked out OK, but with this code in place the display writing works correctly.

If necessary I can leave the extra code in place, but I cannot see why this happens.
Need some more information about where these snippets are located. Are they all in the same function? Is the first snippet outside of a function, giving it external linkage (i.e., making it global)?

Can you strip the original, problematic code down to the barest amount that still exhibits the problem?

In the problematic code, have you tried printing out the addresses at which the strings are stored? Do they make sense?
 

Thread Starter

AlbertHall

Joined Jun 4, 2014
12,346
In an earlier version of this program, this array was accessed in multiple places in the program so the declaration was before the main function.
Now it is only accessed in one function so I tried putting the declaration inside that function. If I include static then the problem still exists. Without the static it all seems to work correctly.
Does this make sense to anyone, and are there any disadvantages to doing this?
 

nsaspook

Joined Aug 27, 2009
13,260
Just a WAG.

What's the source for "SSD1306_puts"? If that function is non-blocking maybe, somehow, the scope of some variable or buffer requires 'puts' to complete execution of the previous invocation (with a update screen function maybe) before being executed again.
 

Thread Starter

AlbertHall

Joined Jun 4, 2014
12,346
SSD1306_puts uses I2C to communicate with the display. The I2C is bit bang and so uses __delay_us for the timing.
 

dcbingaman

Joined Jun 30, 2021
1,065
In an earlier version of this program, this array was accessed in multiple places in the program so the declaration was before the main function.
Now it is only accessed in one function so I tried putting the declaration inside that function. If I include static then the problem still exists. Without the static it all seems to work correctly.
Does this make sense to anyone, and are there any disadvantages to doing this?
The best bet is always to encapsulate and hide. Thus having a header file that provides a function that returns the appropriate text via it's index is the best way to go. This keeps the array as static to the corresponding .c file and provides access only but not manipulation capable. It also provides a very easy way to see what in your program it accessing the array because a break point can be set in the function that returns an appropriate string literal.

Attached is an example.
 

Attachments

Last edited:
Top