Well I wouldn't do it that way at all... All the digits would be in a array of chars.. Then you don't even have to ty function pointers at all.... Function pointers are advanced C.. What you are doing will work, but my god, there are easier ways do do it..

char sevenSeg[] = {0x40,0x79,0x22,0x30,0x19,0x12,0x02,0x78,0x00,0x1,0x7f}; will do the job

Then use PORTB = sevenSeg[x]; then add in RB7.... no function pointers...

That looks slick.

What do you mean by add in rb7?
Rb7 I have running the display dots, it needs to cycle independently of the digits

 

What do you mean by add in rb7?
Rb7 I have running the display dots, it needs to cycle independently of the digits

Exactly.. If you write once to the port, apart from getting away from the RMW issue, you will clear PORTB.7 so you just need t keep the RB7 as a bit variable and attach it just before writing to portb… How many seven segment units are you using?

 

Exactly.. If you write once to the port, apart from getting away from the RMW issue, you will clear PORTB.7 so you just need t keep the RB7 as a bit variable and attach it just before writing to portb… How many seven segment units are you using?

3 segments.
Heres the hardware. Very simple.

 

Digging deeper into why my data memory was consumed so badly. This chip only has 25 bytes data memory.
Since the 7 segment setting array is really an array of non variable variables (static variables?) I can put it in program memory, by setting the array like this;

const char (seg7set[11]) = {
    0b00001000, //0
    0b01101011, //1
    0b01001000, //2
    0b01000001, //3
    0b00100011, //4
    0b00010001, //5
    0b00010000, //6
    0b01001011, //7
    0b00000000, //8
    0b00000001, //9
    0b01111111, //clear
};

And, as far as adding the RB7 bit, I couldnt figure any other way but doing it like this. Seems cumbersome, but it works.

 pba=0b10000000;
 portbset = seg7set[0]+pba;
 PORTB= portbset ;

Then I can change variable pba as required (when i flip the bit on RB7, immediately after i set pba to the correct value), then whenever I change the 7 segment display, it immediately adds pba to the setting, then writes that all to the port.

This is a fun hobby.

 

And, as far as adding the RB7 bit, I couldnt figure any other way but doing it like this. Seems cumbersome, but it works.

Actually, that's fine. You can also use logicals which might compile to smaller code. Use a #define so that you don't have to store pba as another variable:

#define DP 0b10000000  // defines DP seg location on PORTB
unsigned char SegsOut;  // temp variable to retrieve segment pattern, all segment patterns have DP bit == 0
bit DecimalOut; // 1 = append DP to segment pattern for current digit

const char (seg7set[11]) = {  // 0 = segment or DP is ON
    0b10001000, //0
    0b11101011, //1
    0b11001000, //2
    0b11000001, //3
    0b10100011, //4
    0b10010001, //5
    0b10010000, //6
    0b11001011, //7
    0b10000000, //8
    0b10000001, //9
    0b11111111, //clear
};


DecimalOut = 1;  // display 3.
digit_value = 3;   // from your calculations
SegsOut = seg7set[digit_value]; // fetch segment pattern to display '3'
if(DecimalOut)  // convert logical decimal point to IO DP segment logic level
  SegsOut &= ~DP;  // ANDs the segments in LS7bits with 0b01111111 to clear the DP bit to 0 to turn it on
PORTB = SegsOut;  // then output as one byte
// then turn on the digit select for that digit.

Note that you have a logic sense problem in your original segment table vs. the DP bit. If a 0 turns on the segment, you should set the DP bit (turn it off) in each pattern as I've shown. Then for any digit with segment ON, AND it with the mask to clear the DP bit set that output line LOW. You can do it any way you want as long as the logic eventually works but it makes it more complicated to understand.

Other than that, you pretty much have the idea.
Have fun!

 

The basic idea of what I want to do, is have a function or a variable that returns or is a number, and then I want to use that number to directly call the function that holds all the IO settings to set a digit on the 7 segment. I'm stuck here only because the code works on the 16F1564, and does not compile for the 16F54, using the same software and compiler.

You're code is very clumsy, very 1-dimensional.... That's not a jab. My point is, it gives a good idea to how you approach a problem, and how you understand the problem to begin with. You seem to have a better handle on the actual electronics, than the code.

Here is what you're missing- Instead of doing everything brute force, use loops and counters.

As for using a single digit to indicate what function to call-- that makes each digit an index, and you could use an array of function pointers. So long as the arguments are identical for each function.... Or you could use a switch-case statement inside a single function to set up parameters and then a couple of loops do the actual work....

Flow chart. If you flow-chart, it will help you simplify because it gets details out of your head, and lets your mind work. And it lets you express your logic and find errors. You can run your logic on paper, changing values, and stepping through the logic to:

Find missing pieces
Find redundant pieces
See relationships allowing you to write better, more concise code
Identify Logic Errors
...
and more

Teach your kid how to flow-chart as well. he may not use it much- people tend to not want to these day, but if he needs to, he'll know how, and it can make a huge difference in savings of time, effort, and problem solving.

 

You're code is very clumsy, very 1-dimensional.... That's not a jab. My point is, it gives a good idea to how you approach a problem, and how you understand the problem to begin with. You seem to have a better handle on the actual electronics, than the code.

Here is what you're missing- Instead of doing everything brute force, use loops and counters.

As for using a single digit to indicate what function to call-- that makes each digit an index, and you could use an array of function pointers. So long as the arguments are identical for each function.... Or you could use a switch-case statement inside a single function to set up parameters and then a couple of loops do the actual work....

Flow chart. If you flow-chart, it will help you simplify because it gets details out of your head, and lets your mind work. And it lets you express your logic and find errors. You can run your logic on paper, changing values, and stepping through the logic to:

Find missing pieces
Find redundant pieces
See relationships allowing you to write better, more concise code
Identify Logic Errors
...
and more

Teach your kid how to flow-chart as well. he may not use it much- people tend to not want to these day, but if he needs to, he'll know how, and it can make a huge difference in savings of time, effort, and problem solving.

I'm probably building this in a counterintuitive manner. The next step is to incorporate a button with a press release and press hold function. I've been writing out how this works and it seems as though it will be best to use some loops.

My code does use counters presently. Both to multiplex the 7 segments at a determined rate and to give me a 1s clock.

As far as loops go, my training has been mechanical engineering, so I've covered basic electronics in depth, but computer programming is like Swahili to me. Implementing loops and switch-case statements are, well unlearned things at this point.

Appreciate the comments very much. Constructive criticism like this from people who actually read my code is invaluable. It makes learning much quicker. Thank you

 

Actually, that's fine. You can also use logicals which might compile to smaller code. Use a #define so that you don't have to store pba as another variable:

#define DP 0b10000000  // defines DP seg location on PORTB
unsigned char SegsOut;  // temp variable to retrieve segment pattern, all segment patterns have DP bit == 0
bit DecimalOut; // 1 = append DP to segment pattern for current digit

const char (seg7set[11]) = {  // 0 = segment or DP is ON
    0b10001000, //0
    0b11101011, //1
    0b11001000, //2
    0b11000001, //3
    0b10100011, //4
    0b10010001, //5
    0b10010000, //6
    0b11001011, //7
    0b10000000, //8
    0b10000001, //9
    0b11111111, //clear
};


DecimalOut = 1;  // display 3.
digit_value = 3;   // from your calculations
SegsOut = seg7set[digit_value]; // fetch segment pattern to display '3'
if(DecimalOut)  // convert logical decimal point to IO DP segment logic level
  SegsOut &= ~DP;  // ANDs the segments in LS7bits with 0b01111111 to clear the DP bit to 0 to turn it on
PORTB = SegsOut;  // then output as one byte
// then turn on the digit select for that digit.

Note that you have a logic sense problem in your original segment table vs. the DP bit. If a 0 turns on the segment, you should set the DP bit (turn it off) in each pattern as I've shown. Then for any digit with segment ON, AND it with the mask to clear the DP bit set that output line LOW. You can do it any way you want as long as the logic eventually works but it makes it more complicated to understand.

Other than that, you pretty much have the idea.
Have fun!

The dp bit your referring to is actually the colon between the minutes and seconds. It's on 50% of the time, on a 1s interval. It made logical sense to me that if it's on half the time either on or off to start is irrelevant.
Later I'll post my full current code.

 

Well I wouldn't do it that way at all... All the digits would be in a array of chars.. Then you don't even have to ty function pointers at all.... Function pointers are advanced C.. What you are doing will work, but my god, there are easier ways do do it..

char sevenSeg[] = {0x40,0x79,0x22,0x30,0x19,0x12,0x02,0x78,0x00,0x1,0x7f}; will do the job

Then use PORTB = sevenSeg[x]; then add in RB7.... no function pointers...

From a simplification standpiont, I would work with entire bytes, rather than bit fiddling for the port of choice (an 8-bit, aka 1-byte) value to set segments...?

PORTC shown as it's available for I/O for 8-bits:

Assuming the DIP version as it's bread-boardable (Through hole):

 

From a simplification standpiont, I would work with entire bytes, rather than bit fiddling for the port of choice (an 8-bit, aka 1-byte) value to set segments...?

PORTC shown as it's available for I/O for 8-bits:

Assuming the DIP version as it's bread-boardable (Through hole):

View attachment 209220

My chip is a 16f54. Port C does not exist.

 

My chip is a 16f54. Port C does not exist.

I beg your pardon.... This, I believe is yours from the datasheet:


From the above, I take it pins 6-13 are Port B. I had copied the wrong image before. How many 7-segment digits are you driving? Because this MCU has so few pins, it might be worthwhile to consider using 2 pins- one for signal, and one for clock, and controlling a shift-register or something if you're doing more than 1 7-segment display.

 

I beg your pardon.... This, I believe is yours from the datasheet:

View attachment 209262
From the above, I take it pins 6-13 are Port B. I had copied the wrong image before. How many 7-segment digits are you driving? Because this MCU has so few pins, it might be worthwhile to consider using 2 pins- one for signal, and one for clock, and controlling a shift-register or something if you're doing more than 1 7-segment display.

I'm driving 3 7 segments, multiplexed using two pins from PortA. I have a push button, a crystal, and one free pin for future use.
I plan on using the free pin to drive a buzzer.
Hardware is already built.

 

@Travm If you are only driving 3 7-segment displays, and you only have 25 bytes of RAM, it only takes 3 Bytes to describe your display dataspace.

You're first post said it won't compile. If it still won't compile what exactly is the error message the compiler is giving?

I know it's a pain, but seriously, please post a schematic- Plus, you can keep it with working code as your 'documentation' for the project. Keep it in a manilla folder - your kid may want it some day. Kids can be pretty proud of things their Dad makes.

Given that this uses 3 bytes for the display (24 segments (includes DP)), you should be able to drive your display using a single, small function, called 3 times (once for each display). The function prototype:

void DisplayDigit(char val,bool man);         // val is 0x00 through 0x09; man is whether mantissa ON/OFF

1. Set up which display is going to be receive output from DisplayDigit() prior to calling the function
2. Call the function with the desired value
3. Repeat

I don't know what 7-segment display you're using so I'm just (for discussion) using the MAN74A which is segmented as shown below.

So, if we passed the function a '6' the following segments would be on: A,F,G,E,C and D. All others off. If we organize a single Byte to hold that like so:

DP | G | F | E | D | C | B | A
-----------------------------
0  | 1 | 1 | 1 | 1 | 1 | 0 | 1  = 0x7D

I made a map for you so you see all segments defined for 0-9:

So you have an array of segment maps for each digit of unsigned char (since all 8 bits are used):

void DisplayDigit(char val,unsigned char man)
   {
   unsigned char    segMap[] = {0x3F,0x06,0x5B,0x5F,0x46,0x6D,0x7D,0x07,0x7F,0x47};
   unsigned char    segPat = 0x00;

   if((val >= 0) && (val <= 9))
      segPat = segMap[val] & (man?0x80:0x00);
else
      segPat = 0x40;        // range check failed, output a hyphen
 
   // Loop here, shifting & masking for each bit in segPat to a single macro to turn segment on or off
   }

After range-checking, you can use the value passed to DisplayDigit() as the index into the segMap[] array which you then AND with 0x80 IF the mantissa flag is set (2nd argument into function).

Then loop through the bits of the appropriate segMap (eg. segMap[val]), and output them to your latching macro. You only need 1 macro, and you need to wrap your mind around masking and bit-logic operations (AND, OR NOT..., etc) and shifting bits left or right so that you can shift and mask to deal with a single bit at a time.

One function, one macro, 3 bytes of RAM used to keep track of your 3-digit values for persistence, because your segMap is on the stack. Please excuse any typos.

 

@BobaMosfet
Schematic.
I'm going to have to digest that post for a while. I have no idea what your suggested code is doing.

I think if you read all of the posts above you'd see that I have the code working for the 7 segment. Although its been pointed out my code may be less than ideal, it works yet.
Happy to hear all the suggestions on other or better ways to do anything.
Unfortunately I've plans to paint my truck this weekend, so I wont be able to work on the next phase of the project until after the weekend.

But the 7 segment display is working as expected.
Again, this is not final code, this is preliminary i'm loading stuff, then burning the chip to see what happens, repeat. so there is no doubt some garbage.

// CONFIG
#pragma config OSC = XT         // Oscillator selection bits (RC oscillator)
#pragma config WDT = OFF         // Watchdog timer enable bit (WDT enabled)
#pragma config CP = OFF         // Code protection bit (Code protection off)

// #pragma config statements should precede project file includes.
// Use project enums instead of #define for ON and OFF.

#define _XTAL_ 4000000
#include <xc.h>

const char (seg7set[10]) = {
    0b00001000, //0
    0b01101011, //1
    0b01001000, //2
    0b01000001, //3
    0b00100011, //4
    0b00010001, //5
    0b00010000, //6
    0b01001011, //7
    0b00000000, //8
    0b00000001, //9
    //0b01111111, //clear
};

void main(void) {
    

    
    TRISA = 0b1010; //RA3 gets tri-stated to turn off AN1 and AN2,
                    //RA1 must be high imp. its connected straight to ground
    TRISB = 0b00000000; //All PortB Pins are I/O
    OPTION = 000111;  //Options - Prescaler set at 1:256
    int dotflash;
    char previousclk = 0;
    char ansel = 0;
    char portbset = 0;
    char pba = 0b10000000;
  
    RA3=1; //Anode 1 and 2 - tristate this pin to turn off
    RB7=1; //Anode 3 start on Anode 3, or the if statement flow doesnt work


 
    while(1) {       
        //clocks
        if (previousclk > TMR0) //advance item clocks here
        {
            dotflash++;
            ansel++;
        }
        else
        {
            previousclk = TMR0;
        }
        
        if (dotflash==4000)  //4000 cycles is approx 1s
        {
            if (RA0==1) RA0=0;
            else if (RA0==0) RA0=1;
            dotflash=0;           
        }
        
        if (ansel>15)  // This is the display multiplexor
        {
            if (RB7==1)
            {
                RB7=0;
                pba=0b00000000;
                TRISA = 0b0010;
                RA3=1;
            }
            else if (RA3==1)
            {
                RA3=0;
            }
            else if (RA3==0)
            {
                TRISA = 0b1010;
                RB7=1;
                pba=0b10000000;
            }
           ansel=0;
        }
// end of display multiplexor
        
       if (RA1==0)
       {
           portbset = seg7set[0]+pba;
           PORTB= portbset ;
       }
       else
       {
        portbset = seg7set[3]+pba; 
      PORTB = portbset;   
       }
        
      
        
        
        
        
    }
    return;
}

The last bit was to test the push-button, and ensure the hardware was functional. Now its on to the random number generator, the countdown logic, and the button press functions.

 

@Travm No worries. What I provided is simpler than it first appears. It just stores the segment on/off patterns in an array. Your code may work, but it's going about it the hard way. What I'm trying to help you see is a much easier and believe it or not simpler way to look at your problem and solve it.

What I've provided will make more sense as you go along.

Meanwhile- perhaps this will help--

Title: Standard C [Quick Ref]
Author(s): P.J.Plauger, Jim Brodie
ISBN: 1-55615-158-6

Title: Algorithms in C, 3rd Ed. [Parts 1-4, Fundamentals, Data Structures, Sorting, Searching]
Author: Robert Sedgewick
ISBN: 0-201-31452-5

 

@Travm No worries. What I provided is simpler than it first appears. It just stores the segment on/off patterns in an array. Your code may work, but it's going about it the hard way. What I'm trying to help you see is a much easier and believe it or not simpler way to look at your problem and solve it.

What I've provided will make more sense as you go along.

Meanwhile- perhaps this will help--

Title: Standard C [Quick Ref]
Author(s): P.J.Plauger, Jim Brodie
ISBN: 1-55615-158-6

Title: Algorithms in C, 3rd Ed. [Parts 1-4, Fundamentals, Data Structures, Sorting, Searching]
Author: Robert Sedgewick
ISBN: 0-201-31452-5

Thanks again.
Quick question though, if you look at my most recently posted code is that not exactly what I've done? Segment on off patterns in an array? Originally I was using functions and pointers and I was roundly chastised for making it harder on myself.

 

@Travm If you are only driving 3 7-segment displays, and you only have 25 bytes of RAM, it only takes 3 Bytes to describe your display dataspace.

You're first post said it won't compile. If it still won't compile what exactly is the error message the compiler is giving?

I know it's a pain, but seriously, please post a schematic- Plus, you can keep it with working code as your 'documentation' for the project. Keep it in a manilla folder - your kid may want it some day. Kids can be pretty proud of things their Dad makes.

Given that this uses 3 bytes for the display (24 segments (includes DP)), you should be able to drive your display using a single, small function, called 3 times (once for each display). The function prototype:

void DisplayDigit(char val,bool man);         // val is 0x00 through 0x09; man is whether mantissa ON/OFF

1. Set up which display is going to be receive output from DisplayDigit() prior to calling the function
2. Call the function with the desired value
3. Repeat

I don't know what 7-segment display you're using so I'm just (for discussion) using the MAN74A which is segmented as shown below.

So, if we passed the function a '6' the following segments would be on: A,F,G,E,C and D. All others off. If we organize a single Byte to hold that like so:

DP | G | F | E | D | C | B | A
-----------------------------
0  | 1 | 1 | 1 | 1 | 1 | 0 | 1  = 0x7D

I made a map for you so you see all segments defined for 0-9:
View attachment 209290
So you have an array of segment maps for each digit of unsigned char (since all 8 bits are used):

void DisplayDigit(char val,unsigned char man)
   {
   unsigned char    segMap[] = {0x3F,0x06,0x5B,0x5F,0x46,0x6D,0x7D,0x07,0x7F,0x47};
   unsigned char    segPat = 0x00;

   if((val >= 0) && (val <= 9))
      segPat = segMap[val] & (man?0x80:0x00);
else
      segPat = 0x40;        // range check failed, output a hyphen

   // Loop here, shifting & masking for each bit in segPat to a single macro to turn segment on or off
   }

After range-checking, you can use the value passed to DisplayDigit() as the index into the segMap[] array which you then AND with 0x80 IF the mantissa flag is set (2nd argument into function).

Then loop through the bits of the appropriate segMap (eg. segMap[val]), and output them to your latching macro. You only need 1 macro, and you need to wrap your mind around masking and bit-logic operations (AND, OR NOT..., etc) and shifting bits left or right so that you can shift and mask to deal with a single bit at a time.

One function, one macro, 3 bytes of RAM used to keep track of your 3-digit values for persistence, because your segMap is on the stack. Please excuse any typos.

back at this,

have you any links to articles that might help me understand what exactly you mean by a macro to loop and bit-shift and mask. How does this set the port bits?
Also how does this only use 3 bytes of ram? your array segmap[]still holds 10 bytes?

 

@Travm - 3 bytes for your display buffer. That isn't the same as the segment map array. The segmap array is kept on the stack and exists only when the function is called, it doesn't stick around. It was only an example of one method.

Each 'port' on your MCU is 8-pins. As such they fit neatly in a 1-byte register (unsigned). This forms the basis for a concept of masking and flags. Masking is a bit pattern that you compare other bit patterns against logically (AND/OR/XOR/NOR, etc). And the result is a new bit pattern which you can look at to see if (normally) it's value is zero (nothing set), or it's positive (something is set). Each bit by itself is a 'flag'. It's either raised (1) or lowered (0). I'm just describing the vernacular you may hear from time to time.

Thus, if you which to set any number of bits in a port at one setting, you can simply create a byte value where only flag-bits representing those pins you want high, and then OR that against the register itself, and stuff that result back into the register- that turns all those pins on, without disturbing other pins, in one go.

Think about an 8-bit register this way. LSB/MSB may flip-flop based on architecture (endianism), but the concept is the same:



In the above, 10000110 is equivalent to: 128+4+2 = 134 decimal. Or 1000 | 0110 = 8 | 6 or 0x86 hex.

If you have a PORT set to 0x86, and you want the 5th and 6th bits ON, you could do this:

10000110 = 0x86 (the value in the PORT
01100000 = 0x60 (the bits you want on)
-----------
11100110 = 0xE6 (the resulting value to set the port to)

Macros are an advanced topic. Macros are a way to use the C/C++ preprocessor to essentially create a code snippet with a define (in some cases, that can look like a function but isn't). It is useful, for purposes of what I've described, for simplifying sourecode, making tedious operations like bit-fiddling and masking much easier and simple to write (easier to remember) and less prone to mistakes because of less typing. Some will WRONGLY tell you macros are dangerous. People who do so are mere amateurs and have insufficient knowledge in the language, and in developing to correctly weigh in on the subject with any opinion at all. C is an extremely powerful language, not just in terms of flexibility, but in how you can utilize the compiler itself and it's expansion capabilities. I would ignore macros until you're further along.

Meanwhile, learn how to use the '<<' and '>>' operator (shift left or shift right) and masking as I've shown above to loop through the bits of a byte.

 

Meanwhile, learn how to use the '<<' and '>>' operator (shift left or shift right) and masking as I've shown above to loop through the bits of a byte.

This is the only part I dont understand. I understand what << and >> do, and primitively how to use them, but what does this have to do with masking and looping?

Also, the books you linked above, the top one is from 1988. Is this still relevant?

 

@Travm

How do you output a single bit out of a byte, without shifting through each bit (using a mask) and using a loop?

The books are always relevant. C hasn't changed. You can get a newer version, but the idea is for you to have a good C reference, and the algorithm book will help you see ways to do things that you currently are unaware of- helps your mind think in larger circles.