problem with header file functions

Go To Last Post
9 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

hello everyone,

 

today i was experimenting with my own header files. i made one c source file which included function definitions for button debouncing. it also contains 2 other functions which loop until the the button is set or clear with the debouncing part included. i added it to my main program which ran fine and i got no errors while compiling. but the program seems to recognize the first two functions and not the loop ones.

this is the source file:

 

#include <avr/io.h>
#include <util/delay.h>
#include "F:\avr projects\myheaders\button.h"
int button_is_clear(sfr,bit)
{
  if(bit_is_clear(sfr,bit))
  {
    _delay_ms(50);
	if(bit_is_clear(sfr,bit))
	return 1;
  }
  return 0;
}
int button_is_set(sfr,bit)
{
if (bit_is_set(sfr,bit))
 _delay_ms(50);
 if (bit_is_set(sfr,bit))
  {
  return 1;
}
return 0;
}
void loop_until_button_is_set(sfr,bit)
{
do
{

}while(button_is_clear(sfr,bit));
}
void loop_until_button_is_clear(sfr,bit)
{
do
{

}while(bit_is_set(sfr,bit));
}

the main program:

#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>
#include "F:\avr projects\myheaders\button.h"
int main()
{
uint16_t time = 0;
uint8_t arcount = 0;
char password[6] = {'.','.','_','.','.'};
char entry[6];
DDRD |= (1 << PD7)|(1 << PD6);
DDRD &= ~(1 << PD3);
PORTD |= 1 << PD3; 
TCCR1B |= (1 << CS11) | (1 << CS10); //clock prescaler is on 64//
int back;
while(1)
{
while (arcount < 5 )
{
do
{
TCNT1 = 0;
}while(button_is_set(PIND,3));
loop_until_button_is_set(PIND,3);
time = TCNT1;
if (time > 11719)
{
entry[arcount] = '_';
}
else
{
entry[arcount] = '.';
}
arcount++;
}
if((password[0] == entry[0])&&(password[1] == entry[1])&&(password[2] == entry[2])&&(password[3] == entry[3])&&(password[4] == entry[4]))
{
PORTD |= (1 << PD7);
_delay_ms(1000);
}
else
{
for(back = 0;back < 30;back++)
{
 PORTD ^= (1 << PD6);
 _delay_ms(200);
}
}
PORTD &= ~(1 << PD7);
PORTD &= ~(1 << PD6);
arcount = 0;
}
}

the header file:

#ifndef BUTTON_H
#define BUTTON_H

int button_is_clear(sfr,bit);
int button_is_set(sfr,bit);
void loop_until_button_is_clear(sfr,bit);
void loop_until_button_is_set(sfr,bit);
#endif

in the main program in the while(arcount<5 ) loop there is the use of a button_is_set function and the loop_until_button_is_set function.the first one executes perfectly but the second one does not. also when i dont use the function directly but type in the source code(do{}while(button_is_clear())) it works fine. it just seems to have a problem in understanding the function by its name. i have two leds one on PD6 and one on PD7. when the button on PD3 is pressed in a specific pattern led on pd6 should light up else the one on pd7 should blink. its an atmega32.

 

PS this time i have certainly added resistors for the leds.

 

thanks for any help.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

also please tell me if there is a c function that can compare two arrays in the order of there elements

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I think you'll find it a lot easier to see what's going on if you format your code with indentation so it's easier to see what's happening.

 

When you say the second one "does not", what exactly happens?

 

 

Anyway, I think I see your problem. The problem is that when you call a function with an argument, you are passing the value of that argument. So if you call loop_until_button_is_set(PIND, x), you are calling the function with the value of PIND at the time of the call. It isn't then checking that register again; it's checking that value you gave it.

 

I would strongly recommend you switch to modern function declarations, such as:

 

loop_until_button_is_set(int reg, int val)

 

 

That will make it easier to see the issue, but in this case, I think you also probably need to change the function. Consider:

 

void loop_until_button_is_set(int *reg, int bit) {

 

(and then use *reg instead of just reg to access the value)

 

then call it with &PIND, so you're giving it the address of the data it should be checking.

 

Also, I don't think it makes sense to have the delay in the raw button_is_set function. There's no reason to delay that check. If you're trying to do debouncing, you should probably do that in a higher level function, not in that underlying bit test. (Of course, I also probably wouldn't bother with a function for a single-bit test.)

 

Last Edited: Sat. Jan 13, 2018 - 07:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just to let yo get a picture of how much the readability increases with good, consistent indentation habits:

 

#include <avr/io.h>
#include <util/delay.h>
#include "F:\avr projects\myheaders\button.h"
int button_is_clear(sfr, bit)
{
   if (bit_is_clear(sfr, bit)) {
      _delay_ms(50);
      if (bit_is_clear(sfr, bit))
         return 1;
   }
   return 0;
}

int button_is_set(sfr, bit)
{
   if (bit_is_set(sfr, bit))
      _delay_ms(50);
   if (bit_is_set(sfr, bit)) {
      return 1;
   }
   return 0;
}

void loop_until_button_is_set(sfr, bit)
{
   do {

   }
   while (button_is_clear(sfr, bit));
}

void loop_until_button_is_clear(sfr, bit)
{
   do {

   }
   while (bit_is_set(sfr, bit));
}

 

#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>
#include "F:\avr projects\myheaders\button.h"

int main()
{
   uint16_t time = 0;
   uint8_t arcount = 0;
   char password[6] = { '.', '.', '_', '.', '.' };
   char entry[6];

   DDRD |= (1 << PD7) | (1 << PD6);
   DDRD &= ~(1 << PD3);
   PORTD |= 1 << PD3;
   TCCR1B |= (1 << CS11) | (1 << CS10); //clock prescaler is on 64//

   int back;

   while (1) {
      while (arcount < 5) {
         do {
            TCNT1 = 0;
         } while (button_is_set(PIND, 3));
         loop_until_button_is_set(PIND, 3);
         time = TCNT1;
         if (time > 11719) {
            entry[arcount] = '_';
         } else {
            entry[arcount] = '.';
         }
         arcount++;
      }

      if ((password[0] == entry[0]) && (password[1] == entry[1])
          && (password[2] == entry[2]) && (password[3] == entry[3])
          && (password[4] == entry[4])) {
         PORTD |= (1 << PD7);
         _delay_ms(1000);
      } else {
         for (back = 0; back < 30; back++) {
            PORTD ^= (1 << PD6);
            _delay_ms(200);
         }
      }
      PORTD &= ~(1 << PD7);
      PORTD &= ~(1 << PD6);
      arcount = 0;
   }
}

 

[For the curious, I did this using indent -kr -i3 -nut filename  , and I also added a few blank lines manually to the second code chunk.]

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:
consistent

You have a slight inconsistency in the placement of an opening brace ...

 

cheeky

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Can't spot it, Andy. Unless you're referring to opening brace for function is on it's own line while others are on the same line as the controlling statement. Which is consistent (according to K&R according to indent).

 

As I said, I didn't do this by hand but used the indent tool/processor. From it's man page:

The  Kernighan & Ritchie style is used throughout their well-known book
       "The C Programming Language".  It is enabled  with  the  ‘-kr’  option.
       The  Kernighan  &  Ritchie  style  corresponds  to the following set of
       options:

            -nbad -bap -bbo -nbc -br -brs -c33 -cd33 -ncdb -ce -ci4 -cli0
            -cp33 -cs -d0 -di1 -nfc1 -nfca -hnl -i4 -ip0 -l75 -lp -npcs
            -nprs -npsl -saf -sai -saw -nsc -nsob -nss

-br puts braces on same line as if-statement etc

 

To also have braces on same line as function definition head(er) the -brf option would be needed. This option is not present for the Kernighan & Ritchie style (-kr).

 

As far as I can see the code formatted is totally consistent with the K&R style.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

FWIW: I ran the code through Artistic Style http://astyle.sourceforge.net/as... using the following parameters:

astyle --style=kr --pad-oper --pad-header --indent-classes --indent-col1-comments

The resulting output was substantially identical to Johan's.

 

Hmm! Perhaps I'll research tidying blank lines tomorrow.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

thank you everyone/ i used the_real_seebs advice and it works. 

and i will remember the indentations also.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Topping up on what the_real_seebs said: The explanation of why you register parameters must be pointers to registers is spelled out in tha avrlibc FAQ: http://www.nongnu.org/avr-libc/u...

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]