EmbeddedRelated.com
Forums
Imagine Conference
   Forums    More Forums    68hc12  

Help with DP256 'C' Code

Started by Toby Merridan ●January 24, 2003
I am a hardware engineer who has managed to cobble together some test code
for this HCS12 project (dp256) basically I have set up a RS232 link back to
windows terminal. I am testing some inputs (SPI bus) and Digital Outputs,
printing some blurb onto the screen when each channel is switched on
(keyboard hex input), I have some status feedback from the digital Outputs
feeding directly into hardwire Digital Inputs, do some comparison testing
and print out more blurb.

Two things I would like help with:

i. Ideas to tidy it up a bit, my C is not to good, would love to see it done
properly!

ii. Addtional functionality, there is an off chip SPI bus, 12 bit ADC
chip(8AI). I would like to read a channel and pump the data out to the RS232
Terminal. Control characters (like the digitals) are to be read from the
keyboard example type Gx where x is the requested channel number. I
understand the method it is just the C code. This will use an unused SPI bus
(1)

Method:

a. Read input command via 232 from keyboard entry ( using existing
proceedure below but with "Gx" additional commands)
b. Enable the digital switch that selects the ADC chip. Set port H bit 3 to
'1'
c. Enable the appropriate channel.Set ports A0,A1 & A2 to 1.
di.Start D to A conversion process. Set Port A bit 7 to '1'.
dii. Wait one SPI clock pulse for acquisition to take place.
diii. Set A to D conversion to hold. Set port A bit 7 to '0'
div. Wait 14 SPI clock pulses until conversion complete (and transfered onto
SPI bus).
e. Read the SPI, and save the data (configure this bus SPI1 same as SPI0
(digital inputs).
f. Disable the digital switch that selects the analogue channel. Set Port H
bit 3 to '0'.
g. Report the SPI data to RS232 output (using existing proceedure).

This is the code thus far::

/***************************************************************************
***/
/*
*/
/*
*/
/* Project : L019 Power Control Unit
*/
/* File Name : L0199040.c.c
*/
/* Author : Toby Merridan
*/
/* Created : January 2003
*/
/* Description : Hardware Test code entry point main()
*/
/***************************************************************************
***/ #include <hidef.h>
#include <stdio.h>
#include "L0199018.h"
/*Global variables*/
#include <6812DP256.H>
/*CPU Register Definitions*/
#include "L0199006.H"
/*Serial Comms definitions*/
#include "L0199041.h"
/*CPU I/O definitions*/
#include "L0199014.h"
/*Digital IO definitions*/

/* Import from L0199041 */
extern void InitialiseCPU(void); /* Import from L0199003 */
extern void PerformRealTimeRoutine(void); #pragma CODE_SEG CodeForcedToPPage33

/***************************************************************************
**/
/* Title : main()
*/
/* Author :
*/
/* Created : November 2002
*/
/* Description : Main program loop
*/
/* Parameters : None
*/
/***************************************************************************
**/

int tt = 0;

void PrintStr(int port, char *str)
{
uint I;

while (*str != 0)
{
SerialPutChar(port,*str);
str++;
I++;
}
}

ulong StrToUlong(char *cmd_str)
{
uint I;
uint J;
ulong ret;

I = 0;
ret = 0;
while ((*cmd_str != 0)&&(I<6))
{
if(*cmd_str <= '9')
{
J = *cmd_str;
J = J - '0';
}
else if (*cmd_str <= 'F')
{
J = *cmd_str;
J = J - 'A';
J = J + 0x0a;
}
else
{
J = *cmd_str;
J = J - 'f';
J = J + 0x0a;
}
ret = ret * 0x10 + J;
cmd_str++;
I++;
}
return ret;
}

void main(void)
{
int count=0;
char cmd_str[20];
char opstr[20];
char *end;
unsigned long int cmd;
int ch;
int serial_port =1;
int dummy;
long int digital;
long int digital_old;
int Chip1Hi,Chip2Hi,Chip1Lo,Chip2Lo;
char str[80];

InitialiseCPU();
dummy= SerialInit(serial_port,RS232,BAUD_9600,NONE,8);
InitDigitalIP(); /*initialise Digital
IP chips*/

while (TRUE)
{

if(count == 0)
{
dummy = ReadDigital(&digital);
if (digital != digital_old)
{
digital_old = digital;
Chip2Lo = digital & 0xFF;
Chip2Hi = digital >> 8 & 0xFF;
Chip1Lo = digital >> 16 & 0xFF;
Chip1Hi = digital >> 24 & 0xFF;
sprintf( str,"Digital I/P Reads =\r\n ");
PrintStr(serial_port, str);
sprintf( str,"chip1Hi = %x Chip1Lo = %x Chip2Hi = %x Chip2Lo =
%x\r\n\n",Chip1Hi,Chip1Lo,Chip2Hi,Chip2Lo);
PrintStr(serial_port, str);
sprintf( str,"Status I/P Reads = \r\n");
PrintStr(serial_port, str);
sprintf( str,"Ch0= %x Ch1= %x Ch2/3= %x Ch4/5= %x Ch6= %x Ch7= %x
Ch8= %x\r\n",STATUS_CHAN_0, STATUS_CHAN_1,STATUS_CHAN_2_3,
STATUS_CHAN_4_5,STATUS_CHAN_6,STATUS_CHAN_7,STATUS_CHAN_8);
PrintStr(serial_port, str);
sprintf( str,"Ch9= %x Ch10 %x Ch11= %x Ch12= %x Ch13= %x Ch14= %x
Ch15= %x\r\n\n\n",STATUS_CHAN_9,STATUS_CHAN_10,STATUS_CHAN_11,
STATUS_CHAN_12,STATUS_CHAN_13,STATUS_CHAN_14,STATUS_CHAN_15);
PrintStr(serial_port, str);
}
}
cmd_str[6]='\n';
ch= SerialGetChar(serial_port);
if (((ch >= '0') && (ch <= '9')) || ((ch >= 'A') && (ch <= 'F')) || ((ch
>= 'a') && (ch <= 'f')))
{
dummy= SerialPutChar(serial_port,ch);
switch(count)
{
case 0:
case 1:
case 2:
case 3:
case 4:
cmd_str[count]= ch;
count ++;
break;
case 5:
default:
cmd_str[5]= ch;
count = 0;
dummy = SerialPutChar(serial_port,'\n');
dummy = SerialPutChar(serial_port,'\r');
cmd = StrToUlong(cmd_str);
/******* INPUT DEMAND and OUTPUT COMMAND Output Channel
0******************************************************/

if (cmd & 0x01) // Input
Switch State test
{
DIGITAL_OUT_0 = 1; // Switch on
DO0 if true
sprintf( str, "Dig Out Ch0 ON\r\n");
PrintStr(serial_port, str);
}
else
{
DIGITAL_OUT_0 = 0; // Switch
off D00 if false
sprintf( str, "Dig Out Ch0 OFF\r\n");
PrintStr(serial_port, str);
} if (cmd & 0x02)
{
DIGITAL_OUT_1 = 1;
sprintf( str, "Dig Out Ch1 ON\r\n");
PrintStr(serial_port, str);
}
else
{
DIGITAL_OUT_1 = 0;
}
if (cmd & 0x04)
{
DIGITAL_OUT_2 = 1;
}
else
{
DIGITAL_OUT_2 = 0;
}
if (cmd & 0x08)
{
DIGITAL_OUT_3 = 1;
}
else
{
DIGITAL_OUT_3 = 0;
}
if (cmd & 0x10)
{
DIGITAL_OUT_4 = 1;
}
else
{
DIGITAL_OUT_4 = 0;
}
if (cmd & 0x20)
{
DIGITAL_OUT_5 = 1;
}
else
{
DIGITAL_OUT_5 = 0;
}
if (cmd & 0x40)
{
DIGITAL_OUT_6 = 1;
}
else
{
DIGITAL_OUT_6 = 0;
}
if (cmd & 0x80)
{
DIGITAL_OUT_7 = 1;
}
else
{
DIGITAL_OUT_7 = 0;
}
if (cmd & 0x0100)
{
DIGITAL_OUT_8 = 1;
}
else
{
DIGITAL_OUT_8 = 0;
}
if (cmd & 0x0200)
{
DIGITAL_OUT_9 = 1;
}
else
{
DIGITAL_OUT_9 = 0;
}
if (cmd & 0x0400)
{
DIGITAL_OUT_10 = 1;
}
else
{
DIGITAL_OUT_10 = 0;
}
if (cmd & 0x0800)
{
DIGITAL_OUT_11 = 1;
}
else
{
DIGITAL_OUT_11 = 0;
}
if (cmd & 0x1000)
{
DIGITAL_OUT_12 = 1;
}
else
{
DIGITAL_OUT_12 = 0;
}
if (cmd & 0x2000)
{
DIGITAL_OUT_13 = 1;
}
else
{
DIGITAL_OUT_13 = 0;
}
if (cmd & 0x4000)
{
DIGITAL_OUT_14 = 1;
}
else
{
DIGITAL_OUT_14 = 0;
}
if (cmd & 0x8000)
{
DIGITAL_OUT_15 = 1;
}
else
{
DIGITAL_OUT_15 = 0;
}
if (cmd & 0x010000)
{
PWM_OUT_0 = 1;
}
else
{
PWM_OUT_0 = 0;
}
if (cmd & 0x020000)
{
PWM_OUT_1 = 1;
}
else
{
PWM_OUT_1 = 0;
}
if (cmd & 0x040000)
{
PWM_OUT_2 = 1;
}
else
{
PWM_OUT_2 = 0;
}
if (cmd & 0x080000)
{
PWM_OUT_3 = 1;
}
else
{
PWM_OUT_3 = 0;
}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 0 ****************************************/
if (DIGITAL_OUT_0 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_0 ==1)
{
sprintf( str, "Dig Out Ch0 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_0 == 0)
{
if (STATUS_CHAN_0 ==0)
{
sprintf( str, "Dig Out Ch0 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_0 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_0 ==1)
{
sprintf( str, "Dig Out Ch0 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_0 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_0 ==0)
{
sprintf( str, "Dig Out Ch0 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 1 ****************************************/
if (DIGITAL_OUT_1 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_1 ==1)
{
sprintf( str, "Dig Out Ch1 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_1 == 0)
{
if (STATUS_CHAN_1 ==0)
{
sprintf( str, "Dig Out Ch1 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_1 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_1 ==1)
{
sprintf( str, "Dig Out Ch1 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_1 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_1 ==0)
{
sprintf( str, "Dig Out Ch1 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 2 & 3 ****************************************/
if (DIGITAL_OUT_2 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_3 ==1)
{
if (STATUS_CHAN_2_3 ==1)
{
sprintf( str, "Dig Out Ch2/3 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_2 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_3 ==0)
{
if (STATUS_CHAN_2_3 ==0)
{
sprintf( str, "Dig Out Ch2/3 Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_2 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_3 ==0)
{
if (STATUS_CHAN_2_3 ==1)
{
sprintf( str, "Dig Out Ch2/3 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_2 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_3 ==1)
{
if (STATUS_CHAN_2_3 ==0)
{
sprintf( str, "Dig Out Ch3 Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_2 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_3 ==1)
{
if (STATUS_CHAN_2_3 ==1)
{
sprintf( str, "Dig Out Ch2/3 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_2 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_3 ==0)
{
if (STATUS_CHAN_2_3 ==0)
{
sprintf( str, "Dig Out Ch2 Open Load\r\n");
// Send out Open Load message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_2 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_3 ==0)
{
if (STATUS_CHAN_2_3 ==1)
{
sprintf( str, "Dig Out Ch2/3 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_2 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_3 ==1)
{
if (STATUS_CHAN_2_3 ==0)
{
sprintf( str, "Dig Out Ch2/3 at least one output Open
Load\r\n"); // Send out Fault message to RS232 port
PrintStr(serial_port, str);
}
}
}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 4 & 5 ****************************************/
if (DIGITAL_OUT_4 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_5 ==1)
{
if (STATUS_CHAN_4_5 ==1)
{
sprintf( str, "Dig Out Ch4/5 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_4 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_5 ==0)
{
if (STATUS_CHAN_4_5 ==0)
{
sprintf( str, "Dig Out Ch4/5 Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_4 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_5 ==0)
{
if (STATUS_CHAN_4_5 ==1)
{
sprintf( str, "Dig Out Ch4/5 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_4 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_5 ==1)
{
if (STATUS_CHAN_4_5 ==0)
{
sprintf( str, "Dig Out Ch5 Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_4 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_5 ==1)
{
if (STATUS_CHAN_4_5 ==1)
{
sprintf( str, "Dig Out Ch4/5 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_4 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_5 ==0)
{
if (STATUS_CHAN_4_5 ==0)
{
sprintf( str, "Dig Out Ch4 Open Load\r\n");
// Send out Open Load message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_4 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_5 ==0)
{
if (STATUS_CHAN_4_5 ==1)
{
sprintf( str, "Dig Out Ch4/5 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}
}

if (DIGITAL_OUT_4 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (DIGITAL_OUT_5 ==1)
{
if (STATUS_CHAN_4_5 ==0)
{
sprintf( str, "Dig Out Ch4/5 at least one output Open
Load\r\n"); // Send out Fault message to RS232 port
PrintStr(serial_port, str);
}
}
}
/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 6 ****************************************/
if (DIGITAL_OUT_6 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_6 ==1)
{
sprintf( str, "Dig Out Ch6 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_6 == 0)
{
if (STATUS_CHAN_6 ==0)
{
sprintf( str, "Dig Out Ch6 Opoen Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_6 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_6 ==1)
{
sprintf( str, "Dig Out Ch6 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_6 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_6 ==0)
{
sprintf( str, "Dig Out Ch6 Overheat Detected\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 7 ****************************************/
if (DIGITAL_OUT_7 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_7 ==1)
{
sprintf( str, "Dig Out Ch7 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_7 == 0)
{
if (STATUS_CHAN_7 ==0)
{
sprintf( str, "Dig Out Ch7 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_7 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_7 ==1)
{
sprintf( str, "Dig Out Ch7 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_7 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_7 ==0)
{
sprintf( str, "Dig Out Ch7 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 8 ****************************************/
if (DIGITAL_OUT_8 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_8 ==1)
{
sprintf( str, "Dig Out Ch8 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_8 == 0)
{
if (STATUS_CHAN_8 ==0)
{
sprintf( str, "Dig Out Ch8 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_8 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_8 ==1)
{
sprintf( str, "Dig Out Ch8 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_8 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_8 ==0)
{
sprintf( str, "Dig Out Ch8 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 9 ****************************************/
if (DIGITAL_OUT_9 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_9 ==1)
{
sprintf( str, "Dig Out Ch9 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_9 == 0)
{
if (STATUS_CHAN_9 ==0)
{
sprintf( str, "Dig Out Ch9 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_9 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_9 ==1)
{
sprintf( str, "Dig Out Ch9 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_9 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_9 ==0)
{
sprintf( str, "Dig Out Ch9 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 10 ****************************************/
if (DIGITAL_OUT_10 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_10 ==1)
{
sprintf( str, "Dig Out Ch10 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_10 == 0)
{
if (STATUS_CHAN_10 ==0)
{
sprintf( str, "Dig Out Ch10 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_10 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_10 ==1)
{
sprintf( str, "Dig Out Ch10 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_10 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_10 ==0)
{
sprintf( str, "Dig Out Ch10 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 11 ****************************************/
if (DIGITAL_OUT_11 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_11 ==1)
{
sprintf( str, "Dig Out Ch11 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_11 == 0)
{
if (STATUS_CHAN_11 ==0)
{
sprintf( str, "Dig Out Ch11 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_11 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_11 ==1)
{
sprintf( str, "Dig Out Ch11 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_11 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_11 ==0)
{
sprintf( str, "Dig Out Ch11 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 12 ****************************************/
if (DIGITAL_OUT_12 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_12 ==1)
{
sprintf( str, "Dig Out Ch12 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_12 == 0)
{
if (STATUS_CHAN_12 ==0)
{
sprintf( str, "Dig Out Ch12 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_12 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_12 ==1)
{
sprintf( str, "Dig Out Ch12 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_12 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_12 ==0)
{
sprintf( str, "Dig Out Ch12 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 13 ****************************************/
if (DIGITAL_OUT_13 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_13 ==1)
{
sprintf( str, "Dig Out Ch13 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_13 == 0)
{
if (STATUS_CHAN_13 ==0)
{
sprintf( str, "Dig Out Ch13 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_13 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_13 ==1)
{
sprintf( str, "Dig Out Ch13 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_13 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_13 ==0)
{
sprintf( str, "Dig Out Ch13 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 14 ****************************************/
if (DIGITAL_OUT_14 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_14 ==1)
{
sprintf( str, "Dig Out Ch14 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_14 == 0)
{
if (STATUS_CHAN_14 ==0)
{
sprintf( str, "Dig Out Ch14 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_14 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_14 ==1)
{
sprintf( str, "Dig Out Ch14 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_14 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_14 ==0)
{
sprintf( str, "Dig Out Ch14 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

/*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
Channel 15****************************************/
if (DIGITAL_OUT_15 == 1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_15 ==1)
{
sprintf( str, "Dig Out Ch15 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}
}

if (DIGITAL_OUT_15 == 0)
{
if (STATUS_CHAN_15 ==0)
{
sprintf( str, "Dig Out Ch15 Overheat or Switch Failure\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_15 == 0)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_15 ==1)
{
sprintf( str, "Dig Out Ch15 O.K\r\n");
// Send out O.K message to RS232 port
PrintStr(serial_port, str);
}

}

if (DIGITAL_OUT_15 ==1)
// Compare Digital Output State with Digital Output Feedfeedback input
{
if (STATUS_CHAN_15 ==0)
{
sprintf( str, "Dig Out Ch15 Overheat or Open Load\r\n");
// Send out Fault message to RS232 port
PrintStr(serial_port, str);
}

}
//end case
}
}
}
} Port definitions etc: /***************************************************************************
***/
/* /*
*/
/* Project : L019 Power Control Unit
*/
/* File Name : L0199004.h
*/
/* Author :
*/
/* Created : 12th November 2002
*/
/* Description : CPU core Hardwre definitions
*/
/***************************************************************************
***/ /***************************************************************************
***/
/* CPU Hardware definitions
*/
/***************************************************************************
***/

#ifdef PCU
/*Hardware Consts for PCU hardware*/
#define OSC_CLK 10000000
/*Oscillator clock 10MHz*/
#define SYS_CLK 10000000
/*System Clock 50MHz*/
#define BUS_CLK 5000000
// #define OSC_CLK 16000000
/*Oscillator clock 16MHz*/
// #define SYS_CLK 16000000
/*System Clock 16MHz*/
// #define BUS_CLK 8000000
/*Bus clock 8MHz*/
/*Bus clock 25MHz*/
#else
/*************************Hardware Consts for Evaluation
Board*****************/

#define OSC_CLK 16000000
/*Oscillator clock 16MHz*/
#define SYS_CLK 16000000
/*System Clock 16MHz*/
#define BUS_CLK 8000000
/*Bus clock 8MHz*/
#endif

/***********************CPU I/O switch output pin
definitions******************/

#pragma MESSAGE DISABLE C1106
/*Non-standard bitfield type */ extern volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_A @0x00;

extern volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_B @0x01;

extern volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_J @0x268;

extern volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_K @0x32;

extern volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_M @0x250;

extern volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_P @0x258;

extern volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_T @0x240;

#pragma MESSAGE WARNING C1106
/*Non-standard bitfield type */

#define DIG_IP_CS CPU_PORT_B.bit7

#define DIGITAL_OUT_0 CPU_PORT_T.bit7
#define DIGITAL_OUT_1 CPU_PORT_B.bit1
#define DIGITAL_OUT_2 CPU_PORT_J.bit0
#define DIGITAL_OUT_3 CPU_PORT_T.bit4
#define DIGITAL_OUT_4 CPU_PORT_J.bit1
#define DIGITAL_OUT_5 CPU_PORT_T.bit2
#define DIGITAL_OUT_6 CPU_PORT_P.bit5
#define DIGITAL_OUT_7 CPU_PORT_P.bit7
#define DIGITAL_OUT_8 CPU_PORT_K.bit3
#define DIGITAL_OUT_9 CPU_PORT_P.bit4
#define DIGITAL_OUT_10 CPU_PORT_P.bit6
#define DIGITAL_OUT_11 CPU_PORT_K.bit7
#define DIGITAL_OUT_12 CPU_PORT_M.bit0
#define DIGITAL_OUT_13 CPU_PORT_K.bit0
#define DIGITAL_OUT_14 CPU_PORT_K.bit2
#define DIGITAL_OUT_15 CPU_PORT_K.bit1

#define PWM_OUT_0 CPU_PORT_P.bit2
#define PWM_OUT_1 CPU_PORT_P.bit3
#define PWM_OUT_2 CPU_PORT_P.bit0
#define PWM_OUT_3 CPU_PORT_P.bit1

#define STATUS_CHAN_0 CPU_PORT_B.bit0
#define STATUS_CHAN_1 CPU_PORT_T.bit6
#define STATUS_CHAN_2_3 CPU_PORT_T.bit5
#define STATUS_CHAN_4_5 CPU_PORT_T.bit3
#define STATUS_CHAN_6 CPU_PORT_K.bit5
#define STATUS_CHAN_7 CPU_PORT_A.bit6
#define STATUS_CHAN_8 CPU_PORT_B.bit3
#define STATUS_CHAN_9 CPU_PORT_K.bit4
#define STATUS_CHAN_10 CPU_PORT_A.bit2
#define STATUS_CHAN_11 CPU_PORT_A.bit1
#define STATUS_CHAN_12 CPU_PORT_A.bit5
#define STATUS_CHAN_13 CPU_PORT_B.bit5
#define STATUS_CHAN_14 CPU_PORT_B.bit4
#define STATUS_CHAN_15 CPU_PORT_B.bit6 /***************************************************************************
**/
/*
*/
/*
*/
/* Project : L019 Power Control Unit
*/
/* File Name : L0199004.c
*/
/* Author : James Hudson
*/
/* Created : 7th November 2002
*/
/* Description : CPU core Hardwre Access Code
*/
/***************************************************************************
**/ /***************************************************************************
**/
/* CPU Register definitions
*/
/***************************************************************************
**/

#define __DECL__6812DP256_H__
/*make this include the definitions */
#include <6812dp256.h>
#include <hidef.h>
/*machine dependent stuff*/ /***************************************************************************
***/
/* FLASH Protection block declaration
*/
/***************************************************************************
***/

typedef unsigned char tUINT8;
typedef unsigned short tUINT16;
typedef struct
{
const tUINT16 key1;
const tUINT16 key2;
const tUINT16 key3;
const tUINT16 key4;
const tUINT16 res1;
const tUINT8 prot3;
const tUINT8 prot2;
const tUINT8 prot1;
const tUINT8 prot0;
const tUINT8 res2;
const tUINT8 security;
}tFlashProt;
#pragma CONST_SEG FLASH_PROT
extern const tFlashProt FlashProtection =
{
0xFFFF,
/* key1 */
0xFFFF,
/* key2 */
0xFFFF,
/* key3 */
0xFFFF,
/* key4 */
0xFFFF,
/* res1 */
0xFF,
/* block 3 protection */
0xFF,
/* block 2 protection */
0xFF,
/* block 1 protection */
0xFF,
/* block 0 protection */
0xFF,
/* res2 */
0xFE,
/* security: unsecured */
};
#pragma CONST_SEG DEFAULT

/***************************************************************************
***/ #pragma CODE_SEG DEFAULT /***************************************************************************
***/
/* Title : InitialiseCPU()
*/
/* Author :
/* Created :
*/
/* Description : Initialises core CPU registers
*/
/* Parameters : None
*/
/***************************************************************************
***/

/**********************CPU I/O switch output pin
definitions*******************/

volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_A @0x00;

volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_B @0x01;

volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_J @0x268;

volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_K @0x32;

volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_M @0x250;

volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_P @0x258;

volatile struct
{
uchar bit0:1;
uchar bit1:1;
uchar bit2:1;
uchar bit3:1;
uchar bit4:1;
uchar bit5:1;
uchar bit6:1;
uchar bit7:1;
}CPU_PORT_T @0x240;

volatile unsigned char MODRR @0x0257;
/*Module routing register*/ #ifdef PCU
/*Code version for PCU hardware*/

void InitialiseCPU(void)
{
IRQEN = 0;
/* disable IRQ*/
MODRR = 0x40;
/* redirect SPI2 port to port H pinout*/
//DDRA = 0x81;
DDRB = 0x82;
/* Port B Direction*/
DDRH = 0x88;
DDRJ = 0x03;
/* Port J Direction*/
DDRK = 0x8f;
/* Port K Direction*/
DDRM = 0x01;
/* Port M Direction*/
DDRP = 0xff;
/* Port P Direction*/
DDRT = 0x94;
/* Port T Direction*/

PORTB = 0x80;
/* Port B OPs off*/
PTJ = 0x00;
/* Port J OPs off*/
PORTK = 0x00;
/* Port K OPs off*/
PTM = 0x00;
/* Port M OPs off*/
PTP = 0x00;
/* Port P OPs off*/
PTT = 0x00;
/* Port T OPs off*/

RTICTL = 0x4b;
/* pre scaler 100 Hz for 10MHz xtal*/
CRGINT = 0x80;
/* enable RTI*/

EnableInterrupts;
}

#else
/*Code version for Evaluation Board*/ void InitialiseCPU(void)
{
IRQEN = 0;
/* disable IRQ*/
DDRB = 0xff;
/* Port B output*/
// RTICTL = 0x7f;
/* pre scaler*/
// CRGINT = 0x80;
/* enabkle RTI*/
EnableInterrupts;
}

#endif
Thanks for you help Toby Merridan


Reply by Andrei Chichak ●January 24, 20032003-01-24
At 09:08 AM 1/24/2003 +0000, you wrote:
>I am a hardware engineer who has managed to cobble together some test code
>for this HCS12 project (dp256) basically I have set up a RS232 link back to
>windows terminal. I am testing some inputs (SPI bus) and Digital Outputs,
>printing some blurb onto the screen when each channel is switched on
>(keyboard hex input), I have some status feedback from the digital Outputs
>feeding directly into hardwire Digital Inputs, do some comparison testing
>and print out more blurb.
>
>Two things I would like help with:
>
>i. Ideas to tidy it up a bit, my C is not to good, would love to see it done
>properly!

Properly? This is the stuff of religious wars, mostly started by hardware
guys that won't be told how to code by mere software guys ;-)

Get yourself a copy of the Indian Hill Style Guide, it's all over by the web.

Never use GOTO statements (as deemed dangerous back in 1969 by the guy who
wrote the FORTRAN compiler).

Never use the default data types (char, int, float...) in embedded systems
(see MISRA guidelines). As you have in some of your code, use INT8U, INT8S,
INT16U, INT16S, that way you always know how big your variable is. Q: How
big is an int? A: It depends.

Global variables are considered bad style. Make them static and provide
routines to alter them. The people that like to code in assembly will now
poke their heads up and bitch about the cost of the subroutine call. I
curse them with a life of tracking down out of range values blowing up
unrelated code.

Get a copy of "C tips and pitfalls" by Koenig.

Get a copy of PC-lint from gimpel.

For the sake of the people that have to maintain your code, or even
yourself in 6 months (or 11 years), be consistent even though it takes a
few extra lines of code. Remember, after you write it, your code will only
be read twice - 1) by the compiler (who doesn't care what it looks like),
and 2) by the person who has to maintain the code. Write it for the
maintainer, it may be you.

>int tt = 0;

const INT tt = 0; /* Unused value, get rid of it, would be picked up by
lint */ >void PrintStr(int port, char *str)
>{
> uint I;

How big is an int? PIC 8 bits, HC12 16 bits, 68332 32 bits. > while (*str != 0)
> {
> SerialPutChar(port,*str);
> str++;
> I++;
> }
>}
>
>ulong StrToUlong(char *cmd_str)

How long is a long? That's a pretty personal question.

Try using CHAR* cmd_str instead of CHAR *cmd_str. It does the same thing,
but it looks like a character pointer type rather than a character type
that happens to be a pointer.

>{
> uint I;
> uint J;

There are a few styles of capitalization around:
variables in lower case, Routines with capitals, TYPES and DEFINES in
all caps.

some_people_like_to_declare_stuff_like_this - Readable but hard to type
othersPreferInterCaps - Readable
lpstrThenTheresMicrosoftsPreferedMethod - Prefix made irrelevant by lint
and hard to read
stbuf - Hey, your identifiers can be more than 6 characters now! Bloody
FORTRAN hacks.

> J = J - 'A';

Subtracting characters considered dangerous.

> if(count == 0)
> {
> dummy = ReadDigital(&digital);
> if (digital != digital_old)
> {
> digital_old = digital;
> Chip2Lo = digital & 0xFF;
> Chip2Hi = digital >> 8 & 0xFF;
> Chip1Lo = digital >> 16 & 0xFF;
> Chip1Hi = digital >> 24 & 0xFF;
>...

Use white space to make large blocks into logical chunks to make code more
readable.

> if (cmd & 0x02)
> {
> DIGITAL_OUT_1 = 1;
> sprintf( str, "Dig Out Ch1 ON\r\n");
> PrintStr(serial_port, str);
> }
> else
> {
> DIGITAL_OUT_1 = 0;
> }

K&R style indents

if (cmd & 0x02) {
DIGITAL_OUT_1 = 1;
sprintf( str, "Dig Out Ch1 ON\r\n");
PrintStr(serial_port, str);
} else {
DIGITAL_OUT_1 = 0;
}

>Thanks for you help >Toby Merridan

No problem...let the flame wars begin...shield up Scotty!

Andrei

-------
Andrei Chichak #200 10835-120 Street
Senior Software Developer Edmonton, Alberta
Pulmonox Medical Inc. Canada
T5H 3P9
(W) (780) 451-3660
(F) (780) 452-0169

Lat: 53 33' 13.548" N
Lon: 113 31' 43.164" W


Reply by zathra_70 ●January 24, 20032003-01-24

Mmmmmmm, flame war...

I think you missed the point of Toby's message.
He wanted you to re-write it, not critique it.

But besides that, you missed the following:

The One True Indentation: 2, 3, 4, 6 or 8 spaces which may or may not
be tabs.

And K&R {brace style} = Krunched & unReadable as it was made for old
orange screens that could only display 20 or so usable lines. Sadly,
it's also part if the GNU style guide.

I though his code was clean compared to a lot of the stuff I've come
across. Not a model of efficiency, but definitely expressive and
explicit.

As for GOTO, my favorite find:

grep -c goto somecfile.c
1

And in the file:
#define jumpto goto /* this file contains only 1 goto */

grep -c jumpto somecfile.c
11 -zathra_70
--- In , Andrei Chichak <acpmiedm@t...> wrote:
> At 09:08 AM 1/24/2003 +0000, you wrote:
> >I am a hardware engineer who has managed to cobble together some
test code
> >for this HCS12 project (dp256) basically I have set up a RS232 link
back to
> >windows terminal. I am testing some inputs (SPI bus) and Digital
Outputs,
> >printing some blurb onto the screen when each channel is switched on
> >(keyboard hex input), I have some status feedback from the digital
Outputs
> >feeding directly into hardwire Digital Inputs, do some comparison
testing
> >and print out more blurb.
> >
> >Two things I would like help with:
> >
> >i. Ideas to tidy it up a bit, my C is not to good, would love to
see it done
> >properly!
>
> Properly? This is the stuff of religious wars, mostly started by
hardware
> guys that won't be told how to code by mere software guys ;-)
>
> Get yourself a copy of the Indian Hill Style Guide, it's all over by
the web.
>
> Never use GOTO statements (as deemed dangerous back in 1969 by the
guy who
> wrote the FORTRAN compiler).
>
> Never use the default data types (char, int, float...) in embedded
systems
> (see MISRA guidelines). As you have in some of your code, use INT8U,
INT8S,
> INT16U, INT16S, that way you always know how big your variable is.
Q: How
> big is an int? A: It depends.
>
> Global variables are considered bad style. Make them static and provide
> routines to alter them. The people that like to code in assembly
will now
> poke their heads up and bitch about the cost of the subroutine call. I
> curse them with a life of tracking down out of range values blowing up
> unrelated code.
>
> Get a copy of "C tips and pitfalls" by Koenig.
>
> Get a copy of PC-lint from gimpel.
>
> For the sake of the people that have to maintain your code, or even
> yourself in 6 months (or 11 years), be consistent even though it
takes a
> few extra lines of code. Remember, after you write it, your code
will only
> be read twice - 1) by the compiler (who doesn't care what it looks
like),
> and 2) by the person who has to maintain the code. Write it for the
> maintainer, it may be you.
>
> >int tt = 0;
>
> const INT tt = 0; /* Unused value, get rid of it, would be picked
up by
> lint */ > >void PrintStr(int port, char *str)
> >{
> > uint I;
>
> How big is an int? PIC 8 bits, HC12 16 bits, 68332 32 bits. > > while (*str != 0)
> > {
> > SerialPutChar(port,*str);
> > str++;
> > I++;
> > }
> >}
> >
> >ulong StrToUlong(char *cmd_str)
>
> How long is a long? That's a pretty personal question.
>
> Try using CHAR* cmd_str instead of CHAR *cmd_str. It does the same
thing,
> but it looks like a character pointer type rather than a character type
> that happens to be a pointer.
>
> >{
> > uint I;
> > uint J;
>
> There are a few styles of capitalization around:
> variables in lower case, Routines with capitals, TYPES and
DEFINES in
> all caps.
>
> some_people_like_to_declare_stuff_like_this - Readable but hard to type
> othersPreferInterCaps - Readable
> lpstrThenTheresMicrosoftsPreferedMethod - Prefix made irrelevant by
lint
> and hard to read
> stbuf - Hey, your identifiers can be more than 6 characters now! Bloody
> FORTRAN hacks.
>
> > J = J - 'A';
>
> Subtracting characters considered dangerous.
>
> > if(count == 0)
> > {
> > dummy = ReadDigital(&digital);
> > if (digital != digital_old)
> > {
> > digital_old = digital;
> > Chip2Lo = digital & 0xFF;
> > Chip2Hi = digital >> 8 & 0xFF;
> > Chip1Lo = digital >> 16 & 0xFF;
> > Chip1Hi = digital >> 24 & 0xFF;
> >...
>
> Use white space to make large blocks into logical chunks to make
code more
> readable.
>
> > if (cmd & 0x02)
> > {
> > DIGITAL_OUT_1 = 1;
> > sprintf( str, "Dig Out Ch1 ON\r\n");
> > PrintStr(serial_port, str);
> > }
> > else
> > {
> > DIGITAL_OUT_1 = 0;
> > }
>
> K&R style indents
>
> if (cmd & 0x02) {
> DIGITAL_OUT_1 = 1;
> sprintf( str, "Dig Out Ch1 ON\r\n");
> PrintStr(serial_port, str);
> } else {
> DIGITAL_OUT_1 = 0;
> }
>
> >Thanks for you help
> >
> >
> >Toby Merridan
>
> No problem...let the flame wars begin...shield up Scotty!
>
> Andrei
>
> -------
> Andrei Chichak #200 10835-120 Street
> Senior Software Developer Edmonton, Alberta
> Pulmonox Medical Inc. Canada
> T5H 3P9
> (W) (780) 451-3660
> (F) (780) 452-0169
>
> Lat: 53 33' 13.548" N
> Lon: 113 31' 43.164" W




Reply by Bob Furber ●January 24, 20032003-01-24
Thanks Andrei,

Pretty good advice.

> For the sake of the people that have to maintain your
code, or even
> yourself in 6 months (or 11 years), be consistent even
though
> it takes a
> few extra lines of code. Remember, after you write it,
your
> code will only
> be read twice - 1) by the compiler (who doesn't care what
it
> looks like),
> and 2) by the person who has to maintain the code. Write
it for the
> maintainer, it may be you.

If your code is any good, it will likely be maintained,
modified, enhanced or used as sample code ..by someone else.
So, always write it for "the next guy" (who might even be
yourself, years hence).

If your code is ugly, requiring the least bit of reverse
engineering effort, it will be tossed and rewritten ($$$).

So, don't bother writing quick and dirty code. At best it
will be a wasted effort. At worst it could waste someone
else's effort trying to figure out what you have done.

Code should be neatly laid out and a pleasure to view and
read. Like writing and poetry, annotation is an art. Too
much can be worse than too little. When labels are chosen
very thoughtfully, your code becomes almost self
documenting. Your code should read like a captivating
mystery book.

Use lots of white space, line up '=' and "// Comments" so
your code not only is good, but, it looks good, to boot.
..So it looks like it was written by someone who feels
strongly about doing his very best.

Ah, and don't even think of using "******.." lines.

Bfn,

Bob Furber

__________________________________________________________

Connect your micro to the internet the easy way
www.microcommander.com

Microcontroller with an obscenity of I/O & features
..in a small footprint www.steroidmicros.com
__________________________________________________________



Reply by Steve Russell ●January 25, 20032003-01-25
Toby,

The code below looks as if it was generated by pasting a prototype piece of
code and editing it for the particular channel.

Can you write a function that can be called on one line to do the task?

Can you write a table so that the task can be done with a for loop on the
channel number?

Using C arrays and structures would help here. If you're not familiar with
them, read up on them and see if you can find some example code that uses them.

Steve At 01:08 AM 1/24/2003, you wrote:
>...
>i. Ideas to tidy it up a bit, my C is not to good, would love to see it done
>properly!

... > /*********TEST FOR OVERLOAD & SWITCH FAILURE CONDITIONS Output
>Channel 1 ****************************************/ >
> if (DIGITAL_OUT_1 == 1)
>// Compare Digital Output State with Digital Output Feedfeedback input
> {
> if (STATUS_CHAN_1 ==1)
> {
> sprintf( str, "Dig Out Ch1 O.K\r\n");
>// Send out O.K message to RS232 port
> PrintStr(serial_port, str);
> }
> }
>
> if (DIGITAL_OUT_1 == 0)
> {
> if (STATUS_CHAN_1 ==0)
> {
> sprintf( str, "Dig Out Ch1 Overheat or Switch Failure\r\n");
>// Send out Fault message to RS232 port
> PrintStr(serial_port, str);
> }
>
> }
>
> if (DIGITAL_OUT_1 == 0)
>// Compare Digital Output State with Digital Output Feedfeedback input
> {
> if (STATUS_CHAN_1 ==1)
> {
> sprintf( str, "Dig Out Ch1 O.K\r\n");
>// Send out O.K message to RS232 port
> PrintStr(serial_port, str);
> }
>
> }


*************************************************************************
Steve Russell mailto:
Senior Software Design Engineer http://www.nohau.com
Nohau Corporation phone: (408)866-1820
51 East Campbell Avenue fax: (408)378-7869
Campbell, CA 95008
*************************************************************************

Reply by Steve Russell ●January 25, 20032003-01-25
You have an example of how to make a easy to understand description of code
in your Email:

At 01:08 AM 1/24/2003, Toby Merridan wrote:
>...
>Method:
>
>a. Read input command via 232 from keyboard entry ( using existing
>proceedure below but with "Gx" additional commands)
>b. Enable the digital switch that selects the ADC chip. Set port H bit 3 to
>'1'
>c. Enable the appropriate channel.Set ports A0,A1 & A2 to 1.
>di.Start D to A conversion process. Set Port A bit 7 to '1'.
>dii. Wait one SPI clock pulse for acquisition to take place.
>diii. Set A to D conversion to hold. Set port A bit 7 to '0'
>div. Wait 14 SPI clock pulses until conversion complete (and transfered onto
>SPI bus).
>e. Read the SPI, and save the data (configure this bus SPI1 same as SPI0
>(digital inputs).
>f. Disable the digital switch that selects the analogue channel. Set Port H
>bit 3 to '0'.
>g. Report the SPI data to RS232 output (using existing proceedure).

See if you can write a function that does each major step. The function
for step d would probably call 4 sub-functions.

The names of the functions could be a shortened versions of the description
lines above.

Hope this helps.

Steve
*************************************************************************
Steve Russell mailto:
Senior Software Design Engineer http://www.nohau.com
Nohau Corporation phone: (408)866-1820
51 East Campbell Avenue fax: (408)378-7869
Campbell, CA 95008
*************************************************************************

Reply by Steve Russell ●January 25, 20032003-01-25
You have an example of how to make a easy to understand description of code
in your Email:

At 01:08 AM 1/24/2003, Toby Merridan wrote:
>...
>Method:
>
>a. Read input command via 232 from keyboard entry ( using existing
>proceedure below but with "Gx" additional commands)
>b. Enable the digital switch that selects the ADC chip. Set port H bit 3 to
>'1'
>c. Enable the appropriate channel.Set ports A0,A1 & A2 to 1.
>di.Start D to A conversion process. Set Port A bit 7 to '1'.
>dii. Wait one SPI clock pulse for acquisition to take place.
>diii. Set A to D conversion to hold. Set port A bit 7 to '0'
>div. Wait 14 SPI clock pulses until conversion complete (and transfered onto
>SPI bus).
>e. Read the SPI, and save the data (configure this bus SPI1 same as SPI0
>(digital inputs).
>f. Disable the digital switch that selects the analogue channel. Set Port H
>bit 3 to '0'.
>g. Report the SPI data to RS232 output (using existing proceedure).

See if you can write a function that does each major step. The function
for step d would probably call 4 sub-functions.

The names of the functions could be a shortened versions of the description
lines above.

Hope this helps.

Steve *************************************************************************
Steve Russell mailto:
Senior Software Design Engineer http://www.nohau.com
Nohau Corporation phone: (408)866-1820
51 East Campbell Avenue fax: (408)378-7869
Campbell, CA 95008
************************************************************************* *************************************************************************
Steve Russell mailto:
Senior Software Design Engineer http://www.nohau.com
Nohau Corporation phone: (408)866-1820
51 East Campbell Avenue fax: (408)378-7869
Campbell, CA 95008
*************************************************************************

Reply by Steve Russell ●January 25, 20032003-01-25
Toby,

You have gotten a lot of advice, all of it good, as far as I can tell.

What of the advice seems helpful to you?

My favorite examples of good advice that is not helpful are:

RTFM! (Read The F****** Manual!) Almost always good advice, but often not
helpful because the manual is incomprehensible or the reader falls asleep
before getting to the material he or she needs. (Or doesn't have the time
to read and understand 1500 pages carefully.)

Sales force: "We would sell more if the price were lower!"

Almost always true, but not helpful, because they can't give a reliable
estimate about HOW MUCH more for HOW MUCH lower a price.

Anyway, Some comments from you about what seems helpful would be welcome,
and likely will improve the advice you get and the advice we give.

Steve
*************************************************************************
Steve Russell mailto:
Senior Software Design Engineer http://www.nohau.com
Nohau Corporation phone: (408)866-1820
51 East Campbell Avenue fax: (408)378-7869
Campbell, CA 95008
*************************************************************************


Reply by hellfire1272000 ●January 27, 20032003-01-27
--- In , Steve Russell <stever@n...> wrote:
> Toby,
>
> You have gotten a lot of advice, all of it good, as far as I can
tell.
>
> What of the advice seems helpful to you?
>
> My favorite examples of good advice that is not helpful are:
>
> RTFM! (Read The F****** Manual!) Almost always good advice, but
often not
> helpful because the manual is incomprehensible or the reader falls
asleep
> before getting to the material he or she needs. (Or doesn't have
the time
> to read and understand 1500 pages carefully.)
>
> Sales force: "We would sell more if the price were lower!"
>
> Almost always true, but not helpful, because they can't give a
reliable
> estimate about HOW MUCH more for HOW MUCH lower a price.
>
> Anyway, Some comments from you about what seems helpful would be
welcome,
> and likely will improve the advice you get and the advice we give.
>
> Steve >
>
**********************************************************************
***
> Steve Russell mailto:stever@n...
> Senior Software Design Engineer
http://www.nohau.com
> Nohau Corporation phone: (408)
866-1820
> 51 East Campbell Avenue fax: (408)
378-7869
> Campbell, CA 95008
>
**********************************************************************
***

Thanks Steve, Andrei & Bob for your help.

You were right in what you say it seems like its each to their own!

I talked to my boss about some of the points and now an argument has
started round the office about the use of non-standard data types
and the portability of code. My boss likes the use of data pools!!
says that as long as there isn't another way of getting to the data
that should be o.k. He agrees that proper access code should be
written but in most cases is not,necessary.

The only "real" C programmer here, says that using INT8U, INT8S etc
etc makes the code more difficult to read and less portable!

But I would agree with you as there is two of us working on test code
and we are always reminding each other how big an INT, CHAR, LONG is
etc. It is difficult to keep track when you are using structures that
have been predefined for you and the elements are using different bit
lengths 2 bit, 4 bit, 1 byte, 2 bytes all within the same data
structures sometimes...bl**dy confusing!

Keep the advice comming, it is all healthy debate (and help)

Thanks

Toby


Reply by Andrei Chichak ●January 27, 20032003-01-27
At 10:24 AM 1/27/2003 +0000, you wrote:
>Thanks Steve, Andrei & Bob for your help.
>
>You were right in what you say it seems like its each to their own!

Think of C as a great huge gun.

The language was designed for writing operating systems and compilers where
you may have to do things like interpret a random piece of memory as one of
a dozen descriptor blocks depending on some obscure flag. The language
trusts that you know what you are doing.

In the wrong hands, it can probably blow your system to bits faster than
any other language.

Assume for the moment that you have an "int" that you are using for pulse
accumulation. It overflows because it is 16 bits wide and the new guy on
the team thought it was a 32 (he has been coding NT SQL server apps for the
last 18 months). Gosh, I guess we will have to rev the app! Well, that's
not really convenient since we are working on an anti-lock braking system
and we have to recall 100,000 vehicles.

C styles can be free and easy. Is this really appropriate in embedded
systems? I don't think so. The body of evidence is piling up that
programming in a "quick and dirty" manner instead of using some level of
rigor costs a lot of time, money, and potentially lives. Now that things
like the MISRA guidelines, extreme programming, Code Complete, C traps and
pitfalls, and the IEEEs push into Software Engineering are published, one
day you may be liable for your code.

You won't be able to say that there are no applicable rules in programming.
That would be like saying that there are no applicable rules in bridge
building.

Andrei
>Thanks
>
>Toby




Imagine Conference