C problem with HCS08 starter kit. - Added p/n to subject.

karmabobby · ‎03-06-2008

#include <hidef.h> /* for EnableInterrupts macro */
#include "derivative.h" /* include peripheral declarations */

#include <MC9S08GB60.h>
  //This file contains a large number of symbol
  //definitions, so we can use names such as
  //PTFD instead of the real address which is 0x0040.
  //You can open the file to see what symbols are defined.

//DELAY SUBROUTINE

void delay(long int n)
{
long int i;
unsigned int j;

for (i=0; i<n; i++)
for (j=0; j<10000; j++)

{}

}

//MAIN PROGRAM

void main (void)
{
unsigned char count=0;
unsigned char x=0;

//case statement to determine which pattern is being shown

switch (PTAD)
{

If (PTAD<0x04);

{
PTAD = PTAD && 0x01;
}

If (PTAD=0x04);
  {
  PTAD = 0x00;
  }

case 1: PTAD = 0x00;
// Pattern 1 (all on/off)

for (x=1; x<=9; x++)
{

  if (x==1) {PTFD=0x00;}
  if (x==2) {PTFD=0xff;}
  if (x==3) {PTFD=0x00;}
  if (x==4) {PTFD=0xff;}
  if (x==4) {PTFD=0x00;}
  if (x==5) {PTFD=0xff;}
  if (x==6) {PTFD=0x00;}
  if (x==7) {PTFD=0xff;}
  if (x==8) {PTFD=0x00;}
  if (x==9) {PTFD=0xff;}
}
break;

case 2: PTAD = 0x01;
//Pattern 2 (Chasing Lights)

for (x=1; x<=9; x++)
{

  if (x==1) {PTFD=0x55;}
  if (x==2) {PTFD=0xAA;}
  if (x==3) {PTFD=0x55;}
  if (x==4) {PTFD=0xAA;}
  if (x==4) {PTFD=0x55;}
  if (x==5) {PTFD=0xAA;}
  if (x==6) {PTFD=0x55;}
  if (x==7) {PTFD=0xAA;}
    if (x==8) {PTFD=0x55;}
  if (x==9) {PTFD=0xAA;}
}
break;

case 3: PTAD = 0x02;
//Pattern 3 (Ping-Pong Lights)

for (x=1; x<=8; x++)
{

  if (x==1) {PTFD=0x80;}
  if (x==2) {PTFD=0x40;}
  if (x==3) {PTFD=0x20;}
  if (x==4) {PTFD=0x10;}
  if (x==4) {PTFD=0x08;}
  if (x==5) {PTFD=0x04;}
  if (x==6) {PTFD=0x02;}
  if (x==7) {PTFD=0x01;}
  if (x==8) {PTFD=0x00;}
}
break;

case 4: PTAD = 0x03;
//Pattern 4(My Pattern)

for (x=1; x<=9; x++)
{

  if (x==1) {PTFD=0x01;}
  if (x==2) {PTFD=0x05;}
  if (x==3) {PTFD=0x56;}
  if (x==4) {PTFD=0x79;}
  if (x==4) {PTFD=0xAA;}
  if (x==5) {PTFD=0x90;}
  if (x==6) {PTFD=0x25;}
  if (x==7) {PTFD=0x87;}
  if (x==8) {PTFD=0x99;}
  if (x==9) {PTFD=0xEE;}
}
break;
}
SOPT_COPE = 0; //Disable the watchdog timer
PTFDD=0xff; //What does this line do?

for (;

//Loop forever. You should NEVER exit main!
   {

    PTFD = ; //Output to PORTF
    count++; //a counter which is increased here
    delay(1); //And delay some time here.
   }
}

The code above is part of an assignment I have been given on my course. I have to have PTFD flash in different patterns whenever the button(PTAD) has been pressed. There is a delay sub routine and a switch statement in the program. I have decided to increment PTAD by anding it with hex (01) as this will increment by one. I am having problems getting the output displayed, Im pretty new at C and I think I have done an Ok job with my code. If anyone can spot any major mistakes and/or help me make this program more efficient and help me sort out this output problem I would be very grateful.

I also need to link the frequency of the output to the A2D converter. What would be the best way to do this? I have thought about this and thought having the A2D variable linked with the size of the delay loop variable seems like a valid idea. However I am new and as you may well know that a lot of ideas you come up with dont necessarily work

Added p/n to subject.

Message Edited by NLFSJ on 2008-03-10 03:12 PM

karmabobby · ‎03-13-2008

Turn out it was the enabling pull ups that had me stuck, I thought my code was correct. I will look into the A2D converter now. I'll let you know if I succeed or get stuck. Thanks for your help, you have been very patient with my vague explanations.

JimDon · ‎03-13-2008

No problem - you may have learned an important lesson - clarify the goal.

karmabobby · ‎03-10-2008

//DELAY SUBROUTINE

void delay(long int n)
{
long int i;
unsigned int j;

for (i=0; i<n; i++)
for (j=0; j<10000; j++)

{}

}

Selected_Pattern = PTAD;
for (x=1;x<10;x++) // Delay needed as Jim noted!
{
PTAD = Patterns[Selected_Pattern][x];

 delay(1)//this would add the required delay }

I understand how to ask for input from the user with regards to the button

in assembly. I dont know how in C.

I guess an If statement that would fall back into the current display loop

if it was false would be the way to go.

assembly:

ButtonUp:

brclr 0,PTAD,ButtonDown ;branch if button is down

bra ButtonUp

ButtonDown:

;Do whatever is required.

So if this was in C then. ButtonDown would be the equivalent subroutine

where the delay and display are both being executed? ButtonUp would be

the Infinate loop.

As for incrementing PTAD, what would be the best way to do this? Having

another variable that incremented whenever it fell into the desired loop?

I'll have a look and post some code up later. Thanks for the pointers,

its really appreciated.

allawtterb · ‎03-10-2008

karmabobby wrote:

I understand how to ask for input from the user with regards to the button

in assembly. I dont know how in C.

<hr/>

First of all, I don't know what your physical interface to the user is. You seem to be using Port A as an input, but is there a button attached to a pin on port A? Is there several buttons attached to several pins? It is unclear from the code you have posted.

karmabobby wrote:

As for incrementing PTAD, what would be the best way to do this?
<hr/>

What do you want to increment PTAD for? PTAD is a register used to inferface with port A, it is not a variable where you store data. Once again, I don't know what you have hooked up to port A so it is hard to determine what you are wanting to do with it. If you can provide at least a description of how you have things hooked up (best would be a schematic) it would be much easier to help you.

JimDon · ‎03-10-2008

Here is a style tip:

// Use
if( 0x04 == PTFD ) // Fine
{
}

if( 0x04 = PTFD ) // ERROR compiler will find your mistake.
{
}

allawtterb · ‎03-10-2008

JimDon wrote:
if( 0x04 = PTFD ) // ERROR compiler will find your mistake.
{
}

This will produce an error but the way karmabobby used it:

if (PTFD = 0x04)

{

//code in here

}

Will not produce an error, but should produce a warning or notice that the expression is always true.

karmabobby · ‎03-13-2008

#include <hidef.h> /* for EnableInterrupts macro */
#include "derivative.h" /* include peripheral declarations */

unsigned char Selected_Pattern=0;
unsigned char x=0;
const unsigned char Patterns[4][10] =

    {

    {0x00,0xFF,0x00,0xFF,0x00,0xFF,0x00,0xFF,0x00,0xFF}, //"all on/off pattern"
    {0x55,0xAA,0x55,0xAA,0x55,0xAA,0x55,0xAA,0x55,0xAA}, //"chasing lights" pattern
    {0x80,0x40,0x80,0x40,0x80,0x40,0x80,0x40,0x00,0x00}, //"Ping-pong" pattern
    {0x01,0x05,0x56,0x79,0xAA,0x90,0x25,0x87,0x99,0xEE} //"My Pattern" Scrambled

    };

//DELAY SUBROUTINE

void delay(long int n) {

long int i;
unsigned int j;

for (i=0; i<n; i++)
for (j=0; j<10000; j++)

{}

}

void main(void){

EnableInterrupts; /* enable interrupts */
/* include your code here */

SOPT_COPE = 0; //Disable the watchdog timer
PTFDD=0xff;

//const will store this in flash since it is not being modified
// 4 is the number of patterns and 10 is the length of all patterns

for(;

{

for (x=1;x<10;x++) {


    if (PTAD==0) {
      Selected_Pattern = Selected_Pattern++;
    }

    PTFD = Patterns[Selected_Pattern][x];

    delay(1); //called delay subroutine

}
    __RESET_WATCHDOG(); /* feeds the dog */
}/* loop forever */
/* please make sure that you never leave main */

}

PTAD is an active low push button. High when pressed down and low when up.

Heres some assembly code. I dont know how to increment the variable Selected_Pattern by one whenever the button is pressed in C. In my current program it simply cycles through the patterns, and when its changed to a one it does nothing.

karmabobby · ‎03-13-2008

InfLoop: ; infinite loop testing button on PTA1

brset 0,PTAD,ButtonUp ; branch if set, i.e. button up

ButtonDown: ; bit is clear so button is down

bset 0,PTFD ; turn LED on

bra InfLoop ; back round loop again

ButtonUp: ; bit is set so button is up

bclr 0,PTFD ; turn LED off
bra InfLoop ; back round loop again

Message Edited by karmabobby on 2008-03-13 06:25 PM

JimDon · ‎03-13-2008

"PTAD is an active low push button. High when pressed down and low when up."

This statement contradicts itself.

active low means low when you press it.
Which is it?
Also, why are you feeding the dog if you disabled COP.

Is it correct that PTAD has 2 buttons on bits 0 an 1, that are low true, and that which button(s) you push determines which pattern should be displayed?

If this is correct, then try this. It does not take debounce into consideration, but see how well it works.

for(;

{

Selected_Pattern = PTAD & 3; // 0,1,2, or 3

// do the pattern once for the pressed buttons.

for (x=1;x<10;x++) {

    PTFD = Patterns[Selected_Pattern][x];

    delay(1); //called delay subroutine

}
}/* loop forever */
/* please make sure that you never leave main */

}

karmabobby · ‎03-13-2008

Its 0 (low) when pushed and 1 (high) when up. Its only one button with these 2 states. I will try what you have suggested, a debounce subroutine isn't too hard.....well I know how in assembly just not in C yet.

Ignore the feeding the dog, my lecturer uses bizarre analogies when teaching....he says programming is a practical subject and therefore lectures are pretty much useless.

What I was trying to do was to detect when the button is pressed down. I am not sure....I have kind of had a eureka moment though, usually in assembly when this didnt work it was because I had not enabled pull-ups on the pins. I will have a look about this in C for this microcontroller. This would explain the cycling through patterns using one value in the variable and not doing anything with the other. I'll let you know, but keep suggestions coming its very helpful

allawtterb · ‎03-07-2008

First of all, there are 3 things wrong with this if statement:

If (PTAD=0x04);

1) if should be lower case.

2) You don't want to assign PTAD to 0x04, so you should have used "==".

3) You don't put a semi-colon after an if statement unless you don't want it to do anything when it is true.

karmabobby wrote:
I have decided to increment PTAD by anding it with hex (01) as this will increment by one.

This does not increment PTAD by one, after using either a logical( && ) or bit-wise( & ) ampersand you will get the same result for anding with 0x01. PTAD will be 1 if the LSB was set, otherwise it will be 0.

karmabobby wrote:
....
//case statement to determine which pattern is being shown
switch (PTAD)
{
If (PTAD<0x04);
{
PTAD = PTAD && 0x01;
}
If (PTAD=0x04);
  {
  PTAD = 0x00;
  }
....

You can't place these if statements like this instead a switch! Read up on switches, code should only be placed after cases like you did with your for loops.

As for how you are displaying the patterns, how about you use a 2 dimensional array? Since your patterns are almost all the same length this will greatly simply your code. The switch won't be needed and the if statements in the for loops will all be gone. This should be added under your includes:

Code:

// "const" will store this in flash since it is not being modified// 4 is the number of patterns you have and 10 is the length of all patternsconst unsigned char Patterns[4][10] =        {   {0x00,0xFF,0x00,0xFF,0x00,0xFF,0x00,0xFF,0x00,0xFF},  // Your "all on/off pattern"   {0x55,0xAA,0x55,0xAA,0x55,0xAA,0x55,0xAA,0x55,0xAA},  // "Chasing lights" pattern   {0x80,0x40,0x20,0x10,0x08,0x04,0x02,0x01,0x00,0x00},  // "Ping-pong" pattern, Added an extra 0x00 to make them all the same length   {0x01,0x05,0x56,0x79,0xAA,0x90,0x25,0x87,0x99,0xEE}   // Your pattern};

Your switch can be removed and all the code inside remove, just one for loop is needed now but as Jim noted, there should be delays added! Also, I suggest you read PTAD into a local variable (I named it Selected_Pattern)

Code:

 Selected_Pattern = PTAD; for (x=1;x<10;x++)       // Delay needed as Jim noted!  {    PTAD = Patterns[Selected_Pattern][x]; }

You have another problem, you are never waiting for someone to press a button! The code will just run through when run because you are not waiting for any user input!

Get this working before you try doing anything with the ADC.

Message Edited by allawtterb on 2008-03-07 09:55 AM

JimDon · ‎03-07-2008

Here is a hint:

Rather than just tell you, I will tell you how to figure out your problems.
PTFDD=0xff; //What does this line do?

Open the pdf that is the specification for this chip. If you don't have it, download it.

Search on PTFDD and read what that register does.

Also, in your for loops, think about how fast they execute and where you might place a delay.

C problem with HCS08 starter kit. - Added p/n to subject.

C problem with HCS08 starter kit. - Added p/n to subject.

General