Enkla men fatala buggar

[Icecap]

Nu är du väl fel ute!

Jag kan ha samma bit-nummer på två olika portar och då blir enum knappast någon fördel heller.

Mitt exempel var tänkt som den .H-fil man skapar där man beskriver vilken hårdvara som är monterat var. Då är det ganska lätt att ha överblick med den definitionstyp.

[TomasL]

Men du har väl knappast samma namn på bägge portar.
Grejjen är att:
I stället för att använda #define för att skapa namnade konstanter är det bättre att använda enum, eftersom man inte riskerar att definiera om det, utan att få kompileringsfel.

Se det så här

I h-fil nummer 100 har du följande define

#define ETT 1
#define TVÅ 2
#define TRE 3
....
osv

i h-fil nummer 999 har du glömt av att du har gjort den definen, för säg två år sedan eller så.

I denna filen så har du följande i stället
#define ETT 3
#define TVÅ 4
#define TRE 5

Så inkluderar du den nya h-filen i ditt projekt, och allt går åt helvete, eftersom ETT, TVÅ och TRE har blivit omdefinierat.

om du i stället i fil 100 har
enum {ETT=1, TVÅ, TRE}

så får du ett felmeddelande om du försöker omdefiniera dessa konstanter, till exempel i fil 999
enum {ETT=3, TVÅ, TRE}
eller
#define ETT 3
#define TVÅ 4
#define TRE 5

bägge varianter ger ett felmeddelande, till skillnad från det första exemplet.

[jesse]

Det borde ju gälla generellt med #define i så fall, alltså att det aldrig borde användas för att det kan definieras om. Men jag tycker #define är jättesmidigt, så jag tänker fortsätta med det. Min kompilator ger dessutom varningar om man skulle försöka definiera två likadana.

[TomasL]

Jo jag vet att define är smidigt, visst det kan ge varningar, men varningar kan missas, speciellt om det tar någon timme eller mer att bygga, man stirrar ju inte hela tiden på skärmen sas, medans en enom alltid ger ett fel och stoppar kompileringen.
Och ja det gäller alla define naturligtvis.
I vissa lägen är det bra, om man vill ha villkorlig kompilering till exempel, men för att definiera konstanter är det dåligt, och enum är precis lika lätt att använda, tom lättare.

[jesse]

Så här kan det se ut i min kod:

Kod: Markera allt

// general definitions
	#define FALSE 0
	#define TRUE 1

	#define u8 uint8_t
	#define s8 int8_t
	#define u16 uint16_t
	#define s16 int16_t
	#define u32 uint32_t
	#define s32 int32_t

	typedef unsigned char byte;
	typedef unsigned char bool;

	#define b0 (1)
	#define b1 (2)
	#define b2 (4)
	#define b3 (8)
	#define b4 (16)
	#define b5 (32)
	#define b6 (64)
	#define b7 (128)
	#define b(x) (1<<(x))
	
	#define nop() __asm__ __volatile__ ("nop" ::)
	
	#define set(port, val) port |= (val)
	#define reset(port, val) port &= ~(val)

// **** externa input (optokopplare) ****
	
	#define INP_DIS  b(PG0)
	#define INP_FIRE b(PG1)
	#define INP_ARM  b(PG3)
	#define INP_PRIM b(PG4)
	#define INP_SEC  b(PD0)
	#define INP_FOG_SENSE    b(PD2)
	#define INP_PRIME_BUTTON b(PD5)
	
	#define INP_TEST_A b(PF3)
	#define INP_TEST_B b(PF4)
	
	// interna input - övriga
	#define INP_BATT_SWITCH b(PD3)
	#define INP_FLUID_SWITCH b(PD4)
	#define INP_AC_FAIL b(PF6)
	#define VERSION_SELECT b(PD1)			///< 1k motstånd till jord - avläses med intern pull-up vid uppstart.
	
	#define fluid_switch (PIND & INP_FLUID_SWITCH) ///< "TRUE = bruten krets", nollställer datum för vätskebyte
	#define battery_removed  (PIND & INP_BATT_SWITCH ) ///< "TRUE = bruten krets", nollstället datum för batteribyte
	#define prime_button_pressed (!(PIND & INP_PRIME_BUTTON)) ///< "on = sluten krets" = TRUE, testkörning
	#define ac_fail (PINF & INP_AC_FAIL)		///< TRUE = power cut off.
	
	// extern output (reläer, lysdioder)
	#define OUT_AIR_ON b(PC1)				///< aktiveras då pumpar eller luftpump går
	#define OUT_FAULT    b(PC0)				///< aktiveras vid fel
	#define OUT_SERVICE  b(PG2)				///< aktiveras vid servicebehov
	
	#define RS485_DE b(PE2)					///< Aktiverar RS485 sändning
	
	///< intern output (pumpar med mera)
	#define LUFTPUMP  b(PC7)		///< luftpump, 1=on
	#define VENTIL_A  b(PC4)		///< ventil A , 1=on
	#define VENTIL_B  b(PC3)		///< ventil B , 1=on
	#define BATT_LADD b(PB6)		///< utgång till batteriladdare. 1 = on.
	#define HEATER_A  b(PC6)		///< utgång till värmeblock A, 1 = off
	#define HEATER_B  b(PC5)		///< utgång till värmeblock A, 1 = off
	#define DCDC24V   b(PC2)		///< 1 = starta DCDC
	#define BATT_LOAD b(PF7)		///< 1 = ladda ur batteri via 10 ohm mostånd (för test av kapacitet)
		
	// LED output
	#define LED1_PWR b(PD6) ///< "power"
	#define LED2_RDY b(PD7) ///< "ready"
	
	// ***** definiera flöde av vätska **** //
	// fullt kärl, 100% är alltid (100 * 256)
	#define FL_1_4_SEK_100 ((32600.0 * FLUID_30S_FLOW ) /30/4 / FLUID_VOLUME + 0.5) // flöde per 0.25 sekunder - 100% hastighet
	#define FL_1_4_SEK__30 ((32600.0 * FLUID_30S_PRIME) /30/4 / FLUID_VOLUME + 0.5) // flöde per 0.25 sekunder - 30% hastighet
	
	// ************** datastrukurer ************************
	#define MACHINE_NAME_LENGHT  12 ///< antal tecken jämnt delbart med 2.
	#define ANTAL_FELMEDDELANDEN 13 ///< E00-E12
	#define ANTAL_SERVICEMEDDELANDEN 6 ///< S00-S05
	#define ANTAL_LOGGAR 128 ///< totalt antal loggade händelser (max)
	
	#define inp  machineRegisters.inputs
	#define stat machineRegisters.status
	#define input(x) (inp & (x))
	#define status(x) (stat & (x))
		
	//****************************************************************************************


-------------------------------------------------


		// ******************************* ANALOG READINGS (ADC) ********************************************
	
		/** EEPROM-konstanterna krymps ner så de får plats i en u16 (dvs >> 3)	***/
		#define ANALOG(R1,R2) ((((R1)*500.0/(R2)+500)/1024) * (65536 >> 3) + 0.5) ///< R1+R2=späningsdelare, 500 = 5.0 volt*100
		
		/*** ADC omvandlare : ange spänningsdelare R1 och R2					***/
		#define BATT_VOLT_KONSTANT ANALOG(75,33) //(1.5975 * (65536 >> 3) + 0.5) // (batt_volt) sätt konstant (ger värde 0-1636) (16.36 volt)
		#define BATT_CURR_KONSTANT (25000U*2) //(batt_curr) sätt konstant (ger omfång på 25.00 ampere) (1mohm*200V/V:  V=I*0.2 : 25A=5V = 32768)
		#define DC24V_KONSTANT ANALOG(196,33) //(3.39756 * (65536 >> 3) + 0.5)     // (DC24volt)  sätt konstant
		#define AUX_VOLT_KONSTANT ANALOG(33,22) // (33k+22k) --> 12.5 volt max
	

	// ****** bitar i block.blStat *****
	#define B_OK_TEMP		b0	// Blocket fungerar och håller rätt remperatur
	#define B_LOW_TEMP		b1	// Blocket har för låg temp
	#define B_TOO_HIGH		b2	// Blocket har för hög temp
	#define B_BLOCK_ERROR	b6	// sätts om block=aktiv men ej fungerar--> stäng av!
	#define B_ACTIVE		b7	// Blocket existerar //
	
	// ****** bitar i hstatus *******  (b0,b1,b2,b6 måste vara samma i blStat och hstat)
	#define H_OK			b0	// OK att producera dimma NU (något block är OK)
	#define H_LOW_TEMP		b1	// sätts om alla aktiva block har för låg temp
	#define H_OVT_BLOCK		b2	// sätts om något block visar overtemp --> stäng av (desarmera)
	#define H_OVT_PCB		b3	// sätts om PCB visar för hög temp -->	   stäng av (desarmera)
	#define H_ERROR			b6  // sätts om något block är fel
	#define H_FATAL_ERROR	b7	// ALLVARLIGT FEL - desarmera (stäng av!)
	// vid overtemp != OK. Icke aktivt block är aldrig OK //
	
	#define PCB_ERRTEMP -4 * 16 // sätts om PCB temp inte kan läsas in


	#define ADC_BATT_VOLT ADC0D
	#define ADC_BATT_CURR ADC1D
	#define ACD_24_VOLT   ADC2D
	#define ACD_12_VOLT   ADC3D
		
	#define DCDC24V_on()  set(PORTC, DCDC24V)     ///< 0 = starta DCDC
	#define DCDC24V_off() reset(PORTC, DCDC24V)  ///< 1 = stoppa DCDC
	#define battLaddOn()  set(PORTB, BATT_LADD)
	#define battLaddOff() reset(PORTB, BATT_LADD)
	
	#define startFan() system.fanOnTime = ((FAN_ON_TIME) + (FAN_RELEASE_TIME))


	#define set_errormessage(active, nummer) set_message(active, nummer)
	#define set_servicemessage(active, nummer) set_message(active, (nummer) | 0x40)
		
		//*** kz-program *** (max 63 sekunder) *** (u8 / 4)
		#define DEFAULT_DELAY_TIME 1 // sekunder; fördröjning före luft  ** MAX 63s **
		#define DEFAULT_AIR_TIME 30 // sekunder; lufttid ** MAX 63s **
		#define DEFAULT_VALVE_TIME 4  // sekunder; hur länge EN ventil ska vara öppen (och luftpump på)
		#define AFTER_DELAY_TIME   15 // sekunder : tid innan luft kan aktiveras igen.
		
		/********************** vätska **********************************************/
		#define FLUID_AGE_MAX   365*2 // vätska, ålder, max.
		#define FLUID_VOLUME	3000 // ml fullt kärl
		#define FLUID_30S_FLOW	 240 // dimma,   milliliter per 30 sekunder, PER VÄRME BLOCK !
		#define FLUID_30S_PRIME	  48 // priming, milliliter per 30 sekunder, PER VÄRME BLOCK !
		#define  FLUID_LOW_LEVEL  15 // procent av total mängd
		
		#define MIN_SWITCH_TIME 4 // sekunder (batt switch / fluid switch) - för att nollställning ska ske.
		
		/************************  batteri **************************************/
		
		#define BATTERY_CAPACITY 4500 // milliAmpereTimmar
		#define BATTERY_AGE_MAX	  365 // ANTAL DAGAR; batteriets ålder, max.
		
		#define BATTERY_VOLTAGE_MIN		 7.50	// lägsta batterispänning för felmeddelande och pumpstopp
		#define BATTERY_VOLT_MIN_START	11.80	// lägsta batterispänning för felmeddelande i vila + att starta luft
		#define BATTERY_START_CHARGE	12.60	// spänning då laddare startar (12.60)
		#define BATTERY_STOP_CHARGE		13.60	// spänning då laddare stängs av (14.30)
		
		#define BATTERY_CONSIDER_60P	12.60	// spänning under laddning då batteriet anses ha minst 60% laddning
		#define BATTERY_CONSIDER_80P	13.10	// spänning under laddning då batteriet anses ha minst 80% laddning
		
		/************************ BATTERITEST ********************************/
		#define DELAY_BEFORE_TEST		30 //20 // sek
		#define BELASTNING_TID			60//60 // sek
		#define RECOVERY_TIME			30 //20 // sek
		
		#define TIME_AFTER_CHARGE		15  // minuter	(max 255) --- får inte starta test precis efter laddning av batt.
		#define TIME_AFTER_USE			120 // minuter	(max 255) --- -- "" --  efter att batteriet använts (vid power-fail)
		#define MAX_CHARGE_TIME			24 //  timmar	(max 31)--- längre laddtid utan att uppnå OK spänning --> fel!
		#define DAYS_BETWEEN_TESTS		13 // dagar
		
		/***** FLÄKT - KRETSKORT TEMP *****/
		#define PCB_TOO_HOT	 50.0
		#define PCB_HEAT_MAX 40.0
		#define PCB_HEAT_MIN 30.0
		#define FAN_ON_TIME 30  // hur länge fläkten ska blåsa efter dimning:
		//FAN_ON_TIME + FAN_RELEASE_TIME <= 255 sekunder
		#define FAN_RELEASE_TIME 8 // 1 - 100, helst 2-potens: hur lång tid för fläkten att stanna
		#define FAN_ON_POWER 100 // 0-255 utöver grundhastighet !
		#define POWER_UP_FAN_TIME 20 // 0-255 fläkt vid start
		
		/***** VÄRMEBLOCK *****/
		#define BLOCK_T_HIGH 80 	// 330 grader max
		#define BLOCK_T_LOW  65		// 5 grader C lägre än max - då startar uppvärmning igen
		#define BLOCK_T_OVER 90 	// Overtemp limit - stäng av värmen minst 15 minuter. (fördröjningen ej implemeterad)
		//#define BLOCK_T_SLOW 26 	// tempgräns för lägre pumphastighet.
		#define BLOCK_T_MIN  55	 	// Lägsta temp för luftflöde tillåten.
		

vilket senare används i koden:

Kod: Markera allt

void SetLuftpump(u8 ventil)
{
	if (ventil) pumparOff();
	u8 portC = PORTC;
	reset(portC, LUFTPUMP | VENTIL_A | VENTIL_B);
	if (ventil == 1) set(portC, VENTIL_A | LUFTPUMP);
	if (ventil == 2) set(portC, VENTIL_B | LUFTPUMP);
	PORTC = portC;
}

[blueint]

Man kan även göra så här

Kod: Markera allt

#ifdef DOUBLEDEFINED
#error LAMER! LAMER! LAMER! LAMER!
#endif

[TomasL]

Just enum är den rekommenderade metoden i C++, där man avråder från att använda #define

[gkar]

AndLi:
Titta på vilket datablad som helst för AVR/PIC eller varför inte en LCD-display.
Där är varje parameter angiven såhär:

Parameternamn
bit 7 - Den här styr en grej
bit 6 - Den här styr en annan grej
osv...

Har man då en källkod med t.ex parameternamn = 0x47
så är det långt ifrån lika enkelt att se hur man har konfigurerat den här parametern.
Jämfört med om man skrivit parameternamn = 0b01000101

Ofta är det samma sak med värden som hade passat bättre i decimal form än hexadecimal form.
Det förekommer (lyckligtvis) inte så ofta på forumet.

Det är samma trassel varje gång man ska felsöka någon AVR/PIC-kod från nätet (t.ex forumet)
eftersom man måste sitta och "översätta" varenda hex till bin.


Det vore mycket intressant att se någon form av lista över vilka kompilatorer för mikroprocessorer som inte stödjer "0b".
Det binära talsystemet borde väl vara det mest lämpliga med tanke på att man oftast programmerar mot specifika bitar....

Anledningen till att jag, och många andra kör alla binära tal i hex är att man lär sig "se" vad det är i binärt utan problem när man programmerat mycket.
Det andra är att precis som du säger, det är inte standardiserat, dvs det är inte portbar kod.
Portbar kod är ofta ett krav.

När man har längre binära tal, 32 eller 64 bitar, är det inte lika enkelt längre att hålla reda på var i talet du är i binär form tex, 0b00000000000000010000000000000000 dvs 0x00010000, hex är då så mycket kompaktare och lättare att förstå.
Ett bra sätt och kanske bättre än de båda när man har bitadresserade register eller dylikt är att göra "kaka = (1 << 4);" Man ser då direkt vilken bit man är ute efter. Detta kostar inga cykler, kompilatorn löser det enkelt åt dig.

[jesse]

För att undvika buggar och andra fel så är det bra om man skriver snyggt från början. Jag stötte på den här fina lilla stilguiden för C och C++, klart läsvärt för de som inte är helt säkra på hur man ska forma sitt program:

Enkel stilguide för C och C++, av Tommy Olsson

Han har skrivit flera andra guider också, t.ex. en guide för bara C++. Den för C++ är helt klart läsvärd om man jobbar med det språket.

[SvenW]

Tack jesse, det där var en modern och bra stilguide. Den bör alla läsa! Perfekt svenska.

[superx]

Defines är bra ibland, men lätt att missbruka som sagt. En av mina sämre upplevelser med defines var när jag länge och väl försökte förstå varför min inline-assembler inte funkade, till dess att jag upptäckte att preprocessorn hade bytt ut ett instruktionsnamn till en konstant. Ugh!

[bit96]

En viktigt sak att tänka på är att kommentera sin kod.
Och att i kommentaren skriva nåt vettigt!

För att rycka ett exempel i denna tråd:

input = 0x6A
high = input & 0xF0
low = input & 0x0F << 4

..så kommer både high och low att ge 0x60.

Jag vet att ovanstående är ett exempel och jag vill inte anklaga AndLi eller någon annan.
Men om det vore ett skarp läge skulle jag aldrig släppa igenom detta. Och hade jag en anställd som skrev på detta viset, då
Inte bara det att det uppenbart ger fel resultat eftersom parenteser är utlämnade, ingen vet egentligen vad som händer.

Skriv istället nåt i stil med:
input = 0x6A; /* Värde från xxx-sensorn */
high = (input & 0xF0); /* Sensorvärdets högsta 4 bitar är Y-värdet. Behåll i hög nibble för vidare beräkning */
low = (input & 0x0F) << 4; /* Sensorvärdets lägsta 4 bitar är X-värdet. SKIFTA UPP till hög nibble för vidare beräkning */

Observera att jag även satte parenteser för high-värdet. På så sätt blir allt symetriskt och lätt att se.
Dessutom är jag garderad ifall jag behöver göra ändringar, t.ex.:
/* X- och Y-värdet från sensorn ÖKAS med 1. Istället för 0-9 erhålls 1-10 för utskrift */
high = (input & 0xF0)+1; /* Sensorvärdets högsta 4 bitar är Y-värdet. Behåll i hög nibble för vidare beräkning */
low = ((input & 0x0F) << 4)+1; /* Sensorvärdets lägsta 4 bitar är X-värdet. SKIFTA UPP till hög nibble för vidare beräkning */

Här satte jag på extra parenteser för low-värdet för att vara på säkra sidan.
Kommentaren satt jag på egen rad innan eftersom det gäller båda raderna.

Det är inte alltid lätt att hänga med i prioritetsordningen vid beräkningar, men med parenteser blir det som jag vill och inte som nån standard eller kompilatorimplementation vill.

Slutsats:
För att förebygga onödigt buggar så:
1. Kommentera, och kommentera vettigt
2. Gödsla med parenteser.

[blueint]

Vilka operatorer som har högst precedens är väl fastlagt i standarden för C ..? i synnerhet för "&" och "<<".

[bit96]

Det finns skrivet i standarden vilken prioritet alla operatorer har eller INTE har.
Vissa operatorer har samma prioritet och ibland är det definerat som att vara odefinerat.

Dessutom är det vanligt att man ändrar i sin kod.
Dina tankegångar just nu kommer du kanske inte ihåg fem minuter senare eller 6 månader senare.
Ett 'plus' skall in här, en 'shiftning' där o.s.v.
Efter att ha debuggat i några timmar och provat och fipplat och försökt få igång LCD:n ihop med PIC:en blir det till slut en hel del ändringar.

Just därför skall parenteser användas överallt hela tiden.
Även om man kan peka §23.45 sektion f stycke 3 som man tolkar till att det BORDE fungera utan parenteser.

[TomasL]

Dessutom är det omöjligt att veta vad som evalueras först när operatorerna har samma prioritet.