Räkna ut skillnad på två kompass-kurser? Assembler AVR

[Glattnos]

Tänkte berätta att jag fått min sensor nu GY-26 kompass-sensor som skickar riktningen via UART till min AVR.
Jag har även skippat assembler och börjat nöta i C istället, går mycket lättare än jag trodde.

För att räkna ut skillnaden mellan två kurser(räknar egentligen ut hur stor driften är) gör jag såhär:

Kod: Markera allt

volatile short int Kurs_ar;
volatile short int Kurs_old;
volatile short int Drift;
volatile short int Temp;

void update_DRIFT (void)
{
	X = Kurs_ar;
	if (X > 1800) X -= 3600;
	if (Kurs_old > 1800) Kurs_old -= 3600;
	Temp = Kurs_old - X;
	if (Temp < -1800) Temp += 3600;
	if (Temp > 1800) Temp -= 3600;
	Drift = Temp;
	Kurs_old = Kurs_ar;
}

"Kurs_ar" kommer från givaren som talar om vilken kurs man har just "nu" och uppdateras ca 10 ggr i sekunden.
"Drift" är hur mycket som skiljer från senaste gången "Drift" beräknades. Resultat blir + eller - beroende på vilket håll.

Eftersom kursen kommer i form av ascii-tecken(8 byte som ser ut såhär 0x0D, 0x0A, Hundratal, Tiotal, Ental, PUNKT, Tiondelar, Checksum) så har jag gjort såhär:

Kod: Markera allt

#define NEWLINE 0x0A

unsigned char Pointer;
volatile short int Kurs_temp;

ISR(USART_RX_vect)
{
	char ReceivedByte;
	ReceivedByte = UDR0;

	if(ReceivedByte==NEWLINE) Pointer = 1;

	switch(Pointer)
	{
		case 1:
			Pointer++;
			break;
		case 2:
			Pointer++;
			Kurs_temp = ((ReceivedByte - 48) * 1000);
			break;
		case 3:
			Pointer++;
			Kurs_temp += ((ReceivedByte - 48) * 100);
			break;
		case 4:
			Pointer++;
			Kurs_temp += ((ReceivedByte - 48) * 10);
			break;
		case 5:
			Pointer++;
			break;
		case 6:
			Pointer++;
			Kurs_temp += (ReceivedByte - 48);
			Kurs_ar = Kurs_temp;
			break;
		default:
			break;
	}
}

Det verkar fungera bra men det är nog ganska "fel" enligt många. Men jag får i alla fall kursen i "Kurs_ar" utan decimal, jag menar att 180.0 grader blir 1800 osv.
Är det ett problem att allt görs i avbrotts-rutinen?
Bör jag kolla Checksum också?

[jesse]

>ganska "fel" enligt många

Nej, jag tycket det var innovativt. Ovanligt men effektivt

EDIT:

Finns det en checksum bör du kolla det.

Kod: Markera allt

volatile short int Kurs_temp;

Den används väl enbart inuti ISR-rutinen? I så fall är det bättre att göra den till en lokal statisk variabel (den minns då värdet mellan anropen):

Kod: Markera allt

ISR(USART_RX_vect)
{
    static short int Kurs_temp = 0;
... kod...
}

[sodjan]

> Är det ett problem att allt görs i avbrotts-rutinen?

Det är väl "bara" ett av casen som körs vid varje interrupt?
Ser ju inte ut att vara speciellt mycket.

> Bör jag kolla Checksum också?

Det beror ju helt på vad konsekvensen blir om checksumman är
fel och du använder värderna ändå. Är det livshotande så kanske
checksumman bör kollas. Händer ingenting (t.ex därför att det kommer
nya värden en sekund senare) och hela lösningen kan acceptera att man
får ett eller annat felaktigt värde, så kanske det inte spelar någon roll.
Sen finns det nog en skala därimellan som bara du kan bedöma...

[Glattnos]

Okej, tackar! Då blev jag inte helt sågad

jesse: Ja både "Pointer" och "Kurs_temp" används endast i ISR-rutinen så jag kan ju lägga dom som lokala. Men vad blir skillnaden mot att ha dom globala?

Sodjan: Ja det var lite så jag tänkte att det är ju bara ett case som blir kört varje avbrott. Så det tar nog inte så mycket tid i anspråk.

Konsekvensen av ett fel är egentligen inte så stor i detta fall, ungefär lika stora som konsekvensen av att skippa en uträkning(för att checksum inte stämmer).

[Swech]

Skall det vara lite mer hängsle och livrem så bör du ju kolla
att inkommande ascii verkligen är ascii 0 till 9. Nu accepterar den i princip
vad som helst.

Får du ett tecken fel så skall inget värde returneras utan systemet skall vänta på
en ny radmatning och börja om.

Din ISR rutin kan kanske också behöva en flagga som sätts då det verkligen finns nya
beräknade data.

Din huvudloop nollställer då flaggan då den läst av värdet
På så sätt kan du detektera om det plötsligt slutar komma data....

Swech

[Glattnos]

Jag försökte kolla av checksum också, men det vill sig inte. Det står att "calibrate sum" ska vara de 8 lägsta bitarna i resultatet från addition av de 7 första byten. Eller har jag fattat fel, är "calibrate sum" något annat?
Har denna givare:
http://www.elechouse.com/elechouse/imag ... manual.pdf

Kör detta i RX-avbrottet:

Kod: Markera allt

ISR(USART_RX_vect)
{
	char Pointer;
	char Cecksum;
	char ReceivedByte;
	short int Kurs_temp;

	ReceivedByte = UDR0;

	if(ReceivedByte==ENTER) Pointer = 1;

	switch(Pointer)
	{
		case 1:
			Cecksum = ReceivedByte;
			Pointer++;
			break;
		case 2:
			Cecksum += ReceivedByte;
			Pointer++;
			break;
		case 3:
			Cecksum += ReceivedByte;
			Pointer++;
			Kurs_temp = ((ReceivedByte - 48) * 1000);
			break;
		case 4:
			Cecksum += ReceivedByte;
			Pointer++;
			Kurs_temp += ((ReceivedByte - 48) * 100);
			break;
		case 5:
			Cecksum += ReceivedByte;
			Pointer++;
			Kurs_temp += ((ReceivedByte - 48) * 10);
			break;
		case 6:
			Cecksum += ReceivedByte;
			Pointer++;
			break;
		case 7:
			Cecksum += ReceivedByte;
			Pointer++;
			Kurs_temp += ((ReceivedByte - 48) * 1);
		case 8:
			Pointer = 0;
			if(ReceivedByte==Cecksum) Kurs_ar = Kurs_temp;
			break;
		default:
			break;
	}
}

Har jag skrivit någonting fel rent C-mässigt? Om jag tar bort if-satsen i case 8 och bara kör såhär så fungerar det ju:

Kod: Markera allt

		case 8:
			Pointer = 0;
			Kurs_ar = Kurs_temp;
			break;

[labmaster]

Prova med följande:

Kod: Markera allt

int Cecksum;

      case 8:
         Pointer = 0;
         if(ReceivedByte == (char) (Cecksum & 0xff)) Kurs_ar = Kurs_temp;
         break;

Edit: Adderat en typecast

[adent]

Jag tror det är odefinierat om "char" är signed eller unsigned. Så med: "unsigned char" borde det oxo fungera.

Istället för 48 kan du skriva '0' så blir det tydligare.

[Nerre]

En liten fundering

Kod: Markera allt

void update_DRIFT (void)
{
	X = Kurs_ar;
	if (X > 1800) X -= 3600;
	if (Kurs_old > 1800) Kurs_old -= 3600;
	Temp = Kurs_old - X;
	if (Temp < -1800) Temp += 3600;
	if (Temp > 1800) Temp -= 3600;
	Drift = Temp;
	Kurs_old = Kurs_ar;
}

Borde inte den sista raden vara Kurs_old = X? För Kurs_as är ju volatile, den kan ha ändrats mellan X = Kurs_ar och Kurs_old = Kurs_ar?

[labmaster]

Det finns många fördelar med att implementera mottagningen av kursen från kompassen som en så kallad state machine.

Först lite deklararioner, du får själv lägga till det som behövs för att få koden genom kompilatorn.

Kod: Markera allt

#define IDLE_MODE = 0;
#define RUN_MODE = 1;
#define RXSTOP_MODE = 2;
#define NL = 0x0A;
#define CR = 0x0D;

typedef void (*FuncPtr)(void);
FuncPtr StateFunc;

typedef struct {
	short int Kurs_temp;
	short int Kurs_ar;
	int Kurs_old;
	short int Drift;
	int Cecksum;
	unsigned char mode;
} INTERNAL_DATA;

INTERNAL_DATA intData;

Jag tror du kan skippa decimalerna i kursen från kompassen eftersom du inte behöver kursen på tiondelar i denna applikation. Möjligen skulle du kunna runda av enligt gängse avrundningsregler men jag trunkerade för enkelheten skull. Om kompassen skulle hänga sig eller att tecknen inte kommer tillräckligt snabbt kan man även implementera timeout.

Du kan eventuellt behöva lägga till något för hantering av rxStop eftersom jag inte djupstuderat detta i din applikation. rxStart anropas från main eller från den rutin där du startar autopiloten. rxStop anropas där du stänger av autopiloten.

Kod: Markera allt

void startRx(void) {
	GLOBAL_INTERUPT(INT_OFF);

	// Init USART if not already done
	// Init TIMER 0 if not already done

	intData.mode = RUN_MODE;
	StateFunc = rxEnter;
	GLOBAL_INTERUPT(INT_ON);
}

void stopRx(void) {
	GLOBAL_INTERUPT(INT_OFF);

	intData.mode = RXSTOP_MODE;
	GLOBAL_INTERUPT(INT_ON);
}

ISR(USART_RX_vect) {

	GLOBAL_INTERUPT(INT_OFF);

	if (intData.mode != IDLE_MODE) {
		StateFunc();
	}

	GLOBAL_INTERUPT(INT_ON);
}

ISR(TIM0_OVF_vect) {
	if (intData.mode == RUN_MODE) {
		StateFunc = rxEnter;
	}	
}

// State Machine for receiving azimuth from GY-26-USART;

void rxIdle(void) {

}

void rxEnter(void) {

	unsigned char rx;

	rx = UDR0;
	if (rx == CR) {
		intData.Cecksum = rx;
		StateFunc = rxNewLine;
	}
}

void rxNewLine(void) {

	unsigned char rx;

	rx = UDR0;
	if (rx == NL) {
		intData.Cecksum += rx;
		StateFunc = rxHundredsOfAngle;
	} else {
		StateFunc = rxEnter;
	}
}

void rxHundredsOfAngle(void) {

	unsigned char rx;

	rx = UDR0;
	intData.Cecksum += rx;
	intData.Kurs_temp = (rx - 48) * 100;
	StateFunc = rxTensOfAngle;
}

void rxTensOfAngle(void) {

	unsigned char rx;

	rx = UDR0;
	intData.Cecksum += rx;
	intData.Kurs_temp += (rx - 48) * 10;
	StateFunc = rxBitsOfAngle;
}

void rxBitsOfAngle(void) {

	unsigned char rx;

	rx = UDR0;
	intData.Kurs_temp += rx - 48;
	intData.Cecksum += rx;
	StateFunc = rxDecimalPointOfAngle;
}

void rxDecimalPointOfAngle(void) {

	unsigned char rx;

	rx = UDR0;
	intData.Cecksum += rx;
	StateFunc = rxDecimalOfAngle;
}

void rxDecimalOfAngle(void) {

	unsigned char rx;

	rx = UDR0;
	intData.Cecksum += rx;
	// Truncate the decimal point
	StateFunc = rxCrc;
}

void rxCrc(void) {

	unsigned char rx;

	rx = UDR0;
	if ((unsigned char) (intData.Cecksum & 0xff) == rx) {
		intData.Kurs_ar = intData.Kurs_temp;
	}

	if (intData.mode != RXSTOP_MODE) {
		StateFunc = rxEnter;
	} else {
		intData.mode = IDLE_MODE;
		StateFunc = rxIdle;
	}
}

[Swech]

Man kan även försöka undvika fallgropen att en rutin skall göra hela jobbet och därför blir onödigt komplex och svårflörtad.

Gör en separat interrupt som endast samlar in tecken tills dess att CR upptäcks eller man tagit emot x anta tecken.

Signalera till en annan rutin som tar de ihopsamlade tecknen och räknar ut checksum
Stämmer checksum så skickar rutinen de checksum kollade data vidare till ytterligare en rutin vars
enda uppgift är att sortera ut kursdata.

På detta sätt blir varje rutins uppgift väldefinierad och lätt att överskåda samt felsöka.

Swech

[jesse]

Om du nu inte håller på att göra om allt (Så som Swech skriver, t.ex. ) så kan du ju ersätta if-satsen if(ReceivedByte==Cecksum) Kurs_ar = Kurs_temp; med att den sparar skillnaden , så ser du om det är helt slumpartat eller om det är något du ser kanske fattas. Gör en global variabel diff = ReceivedByte - Cecksum; och skriv ut den varje gång du får en ny kurs - eller - varför inte skriva ut både ReceivedByte och Cecksum?

[labmaster]

jesse, om tanken med ditt förslag är att lösa TS problem med checksumman så är det lite overkill eftersom problemet gestaltar sig i att checksumman blir större än 255. Det går således inte att deklarera Cecksum som char.

Lösningen är att ändra deklarationen av Cecksum till int samt maska bort den övre byten och som extra säkerhet typecasta till unsigned char. Givetvis bör han även ändra deklarationen för ReceivedByte till unsigned char.

Implementerar han sedan mottagningen av kompassvinkeln som en "State Machine" så ökar han tillförlitligheten och flexibiliteten ett rejält snäpp.

[jesse]

labmaster: Det ska absolut inte ha någon betydelse. Du adderar 8-bitars tal med 8-bitars tal. resultatet - de 'nedersta' åtta bitarna blir exakt desamma som i ett int eller long int. Att man struntar i att ta hand om den nionde biten (carry) påverkar inte resultatet. Jag gör ofta så och det har aldrig blivit fel. FUngerar oavsett om det är signed eller unsigned dessutom.
255 + 7 = 6.
-1 + 7 = 6.

[Glattnos]

Borde inte den sista raden vara Kurs_old = X? För Kurs_as är ju volatile, den kan ha ändrats mellan X = Kurs_ar och Kurs_old = Kurs_ar?

Absolut! Det har du helt rätt i

jesse: Det hade varit kanon att skriva ut ReceivedByte och Cecksum men nu råkar allt sitta på ett experimentkort och har ingen display. Eventuellt får jag koppla upp det på brädan med en display.
Dock har jag några LED så jag provade detta:

Kod: Markera allt

ISR(USART_RX_vect)
{
	unsigned char Pointer;
	unsigned char ReceivedByte;
	unsigned int Cecksum;
	unsigned int Kurs_temp;

	ReceivedByte = UDR0;

	if(ReceivedByte==ENTER) Pointer = 1;

	switch(Pointer)
	{
		case 1:
			Cecksum = ReceivedByte;
			Pointer++;
			break;
		case 2:
			Cecksum += ReceivedByte;
			Pointer++;
			break;
		case 3:
			Cecksum += ReceivedByte;
			Pointer++;
			Kurs_temp = ((ReceivedByte - 48) * 1000);
			break;
		case 4:
			Cecksum += ReceivedByte;
			Pointer++;
			Kurs_temp += ((ReceivedByte - 48) * 100);
			break;
		case 5:
			Cecksum += ReceivedByte;
			Pointer++;
			Kurs_temp += ((ReceivedByte - 48) * 10);
			break;
		case 6:
			Cecksum += ReceivedByte;
			Pointer++;
			break;
		case 7:
			Cecksum += ReceivedByte;
			Pointer++;
			Kurs_temp += ((ReceivedByte - 48) * 1);
		case 8:
			Pointer = 0;
			if(ReceivedByte == (unsigned char) (Cecksum & 0xff)) PORTB |= 1<<LED1_BIT;//Sätt bit
			else PORTB &= ~(1<<LED1_BIT);//Nollställ bit
			break;
		default:
			break;
	}
}

I det fallet ser man inte ens att LEDen lyser nått så troligtvis är

Kod: Markera allt

(ReceivedByte == (unsigned char) (Cecksum & 0xff))

aldrig sant eller mycket korta stunder. Jag uppdaterar ca 2 ggr i sekunden så den borde lysa en halv sek om det kommer ett tillfälle som den stämmer.

Detta står i data-bladet:

(1)Byte0:0x0D (ASCII: enter)
(2)Byte1:0x0A (ASCII: new line)
(3)Byte2:0x30~0x33 (ASCII: hundreds of angle 0~3)
(4)Byte3:0x30~0x39 (ASCII: tens of angle 0~3)
(5)Byte4:0x30~0x39 (ASCII: bits of angle 0~3)
(6)Byte5:0x2E (ASCII: decimal point of angle)
(7)Byte6: 0x30~0x39 (ASCII: decimal of angle)
(8)Byte7: 0x00~0xFF (calibrate sum)
Byte7= the lower 8 bits of (Byte0+ Byte1+ Byte2+……Byte6)

Gör jag då rätt som lägger adderar byte0......byte6 och jämför med byte7?
Det står ju "calibrate sum" och inte "check sum" eller något liknande. Men det borde ju vara det ändå.