Newsgroup: it.hobby.elettronica.digitale
Titolo: SPI capricciosa

Ciao!
Al solito sono alle prese con i fantasmi.
So che la sfera di cristallo è in riparazione ecc :) però magari sapete 
darmi una dritta su dove cercare il problema.
Ho un modulo Rabbit che comunica via seriale sincrona con un AVR.
Lo schema è quello classico: MISO MOSI incrociati, SCK e SS da Rabbit 
(master) a AVR (slave). Il tutto va a 3.3V e l'AVR ha clock a 20 MHz. La 
SPI viaggia a 1 MHz ma ho provato a scendere molto in frequenza così 
come ad allungare parecchio i tempi tra la selezione dello slave e 
l'inizio del clock. Aggiungo che la configurazione della SPI (CPOL, 
CPHA) è corretta, per scrupolo ho anche provato le altre configurazioni 
ma non riconosce nemmeno un pacchetto - giustamente.
Il problema è questo: ogni 50 ms trasmetto un pacchetto di una 30ina di 
byte. Spesso - a occhiometro 1 su 20 trasmissioni - si verifica una 
corruzione dei dati in quanto il banale crc presente riconosce 
l'inesattezza. E comunque se anche non ne tenessi conto i dati ricevuti 
sono effettivamente errati.
Ho verificato con l'oscilloscopio la correttezza dei segnali e mi 
sembrano ok.
Riporto qui il codice completo del programma, non vorrei avere combinato 
qualche casino così evidente da non vederlo.
#include <avr/io.h>
#include <avr/interrupt.h>
#include <avr/wdt.h>
#include <util/delay.h>
#include "servo.h"
#include "LTC1660.h"
static const unsigned char BitReverseTable256[] =
{
   0x00, 0x80, 0x40, 0xC0, 0x20, 0xA0, 0x60, 0xE0, 0x10, 0x90, 0x50, 
0xD0, 0x30, 0xB0, 0x70, 0xF0,
   0x08, 0x88, 0x48, 0xC8, 0x28, 0xA8, 0x68, 0xE8, 0x18, 0x98, 0x58, 
0xD8, 0x38, 0xB8, 0x78, 0xF8,
   0x04, 0x84, 0x44, 0xC4, 0x24, 0xA4, 0x64, 0xE4, 0x14, 0x94, 0x54, 
0xD4, 0x34, 0xB4, 0x74, 0xF4,
   0x0C, 0x8C, 0x4C, 0xCC, 0x2C, 0xAC, 0x6C, 0xEC, 0x1C, 0x9C, 0x5C, 
0xDC, 0x3C, 0xBC, 0x7C, 0xFC,
   0x02, 0x82, 0x42, 0xC2, 0x22, 0xA2, 0x62, 0xE2, 0x12, 0x92, 0x52, 
0xD2, 0x32, 0xB2, 0x72, 0xF2,
   0x0A, 0x8A, 0x4A, 0xCA, 0x2A, 0xAA, 0x6A, 0xEA, 0x1A, 0x9A, 0x5A, 
0xDA, 0x3A, 0xBA, 0x7A, 0xFA,
   0x06, 0x86, 0x46, 0xC6, 0x26, 0xA6, 0x66, 0xE6, 0x16, 0x96, 0x56, 
0xD6, 0x36, 0xB6, 0x76, 0xF6,
   0x0E, 0x8E, 0x4E, 0xCE, 0x2E, 0xAE, 0x6E, 0xEE, 0x1E, 0x9E, 0x5E, 
0xDE, 0x3E, 0xBE, 0x7E, 0xFE,
   0x01, 0x81, 0x41, 0xC1, 0x21, 0xA1, 0x61, 0xE1, 0x11, 0x91, 0x51, 
0xD1, 0x31, 0xB1, 0x71, 0xF1,
   0x09, 0x89, 0x49, 0xC9, 0x29, 0xA9, 0x69, 0xE9, 0x19, 0x99, 0x59, 
0xD9, 0x39, 0xB9, 0x79, 0xF9,
   0x05, 0x85, 0x45, 0xC5, 0x25, 0xA5, 0x65, 0xE5, 0x15, 0x95, 0x55, 
0xD5, 0x35, 0xB5, 0x75, 0xF5,
   0x0D, 0x8D, 0x4D, 0xCD, 0x2D, 0xAD, 0x6D, 0xED, 0x1D, 0x9D, 0x5D, 
0xDD, 0x3D, 0xBD, 0x7D, 0xFD,
   0x03, 0x83, 0x43, 0xC3, 0x23, 0xA3, 0x63, 0xE3, 0x13, 0x93, 0x53, 
0xD3, 0x33, 0xB3, 0x73, 0xF3,
   0x0B, 0x8B, 0x4B, 0xCB, 0x2B, 0xAB, 0x6B, 0xEB, 0x1B, 0x9B, 0x5B, 
0xDB, 0x3B, 0xBB, 0x7B, 0xFB,
   0x07, 0x87, 0x47, 0xC7, 0x27, 0xA7, 0x67, 0xE7, 0x17, 0x97, 0x57, 
0xD7, 0x37, 0xB7, 0x77, 0xF7,
   0x0F, 0x8F, 0x4F, 0xCF, 0x2F, 0xAF, 0x6F, 0xEF, 0x1F, 0x9F, 0x5F, 
0xDF, 0x3F, 0xBF, 0x7F, 0xFF
};
#define	INVERT_UINT(x)			(BitReverseTable256[x & 0xFF] << 8) | 
(BitReverseTable256[(x >> 8) & 0xFF])
#define	INVERT_BYTE(x)			BitReverseTable256[x]
enum	{FALSE = 0, TRUE};
#define	SPI_BUF_SIZE	29
#define	SPI_TIMEOUT		20
#define	TICK			25
#define	led_on()		PORTB |= _BV(PB0)
#define	led_off()		PORTB &= ~_BV(PB0)
#define	led_toggle()	PORTB ^= _BV(PB0)
#define MAKE_WORD(x) 	((uint16_t) *x << 8) | *(x+1)
uint8_t		digout;
uint8_t		digaux;
uint8_t		hb;
uint16_t	motor[8];
uint16_t	servo[2];
uint16_t	light[2];
uint8_t	volatile SPIbuf[SPI_BUF_SIZE];
uint8_t	volatile SPIidx = 0;
uint8_t	volatile SPIready = FALSE;
uint8_t	volatile SPItimeout = 0;
uint8_t volatile tick;
uint8_t		ch;
void cold_n_dark();
void update_all();
void SPIdecode();
uint8_t crc(uint8_t volatile *s);
ISR(PCINT1_vect) {
	if (bit_is_clear(PINB, PB4)) SPIidx = 0; else SPIready = TRUE;
}
ISR(SPI_STC_vect) {
	if (SPIidx < SPI_BUF_SIZE) SPIbuf[SPIidx++] = SPDR;
}
ISR(TIMER0_COMPA_vect) {
	if (++tick == TICK) {
		tick = 0;
		if (SPItimeout < SPI_TIMEOUT) {
			SPItimeout++;
			update_all();
		} else {
			cold_n_dark();
		}
	}
}
int main(void) {
	/* PORT config */
	DDRA  = 0b11111111;
	PORTA = 0b00000000;
	DDRB  = 0b01001111;
	PORTB = 0b01011111;
	DDRC  = 0b11111111;
	PORTC = 0b00000000;
	DDRD  = 0b11111111;
	PORTD = 0b00001111;
	/* EXT INT config */
	PCMSK1 = _BV(PCINT12);
	PCICR  = _BV(PCIE1);
	/* WDT config */
	wdt_enable(WDTO_500MS);
	/* Timer0 config */
	TCCR0A = _BV(WGM01);
	TCCR0B = _BV(CS02);
	OCR0A  = 155;
	TIMSK0 = _BV(OCIE0A);
	/* PWM Timer2 config */
	TCCR2A = _BV(COM2A1) | _BV(COM2B1) | _BV(WGM21) | _BV(WGM20);
	TCCR2B = _BV(CS22) | _BV(CS21);
	OCR2A  = 0;
	OCR2B  = 0;
	/* SPI Config */
	SPCR  = _BV(SPIE) | _BV(SPE) | _BV(CPOL) | _BV(CPHA);
	/* Init peripherals */
	LTC1660_Init();
	cold_n_dark();
	servo[0] = servo[1] = 512;
	servo_init();
	sei();
	for (;;) {
		wdt_reset();
		if (SPIready) SPIdecode();
	}
}
void cold_n_dark() {
	uint8_t i;
	for (i = 0; i < 8; i++) motor[i] = 0x1FF;
	digout = 0xFF;
	digaux = 0x00;
	hb = 0xFF;
	light[0] = light[1] = 512;
	update_all();
	led_on();
}
void update_all() {
	uint8_t i;
	for (i = 0; i < 8; i++) LTC1660_WriteDAC(i + 1, motor[i]);
	PORTC = digout;
	PORTA = digaux;
	PORTD &= 0xF0;
	PORTD |= hb & 0x0F;
	servo_move(SERVO_A, (int16_t) servo[0] - 512);
	servo_move(SERVO_B, (int16_t) servo[1] - 512);
	OCR2A  = light[0] >> 2;
	OCR2B  = light[1] >> 2;
}
void SPIdecode() {
	uint16_t new_crc, old_crc;
	uint8_t i;
	/*
	*
	* DO HB MOT[0..7] SERVO[0..1] LIGHT[0..1] DAUX
	* 1  1  2         2           1			  1
	*
	*/
	SPIready = FALSE;
	for (i = 0; i < SPI_BUF_SIZE; i++) SPIbuf[i] = INVERT_BYTE(SPIbuf[i]);
	new_crc = crc(SPIbuf);
	old_crc = SPIbuf[28];
	if (new_crc != old_crc) return;
	SPItimeout = 0;
	digout   = SPIbuf[0];
	hb 		 = SPIbuf[1];
	motor[0] = MAKE_WORD(&SPIbuf[2]);
	motor[1] = MAKE_WORD(&SPIbuf[4]);
	motor[2] = MAKE_WORD(&SPIbuf[6]);
	motor[3] = MAKE_WORD(&SPIbuf[8]);
	motor[4] = MAKE_WORD(&SPIbuf[12]);
	motor[5] = MAKE_WORD(&SPIbuf[12]);
	motor[6] = MAKE_WORD(&SPIbuf[14]);
	motor[7] = MAKE_WORD(&SPIbuf[16]);
	servo[0] = MAKE_WORD(&SPIbuf[18]);
	servo[1] = MAKE_WORD(&SPIbuf[20]);
	light[0] = MAKE_WORD(&SPIbuf[22]);
	light[1] = MAKE_WORD(&SPIbuf[24]);
	digaux   = SPIbuf[26];
	led_toggle();
}
uint8_t crc(uint8_t volatile *s) {
	uint8_t volatile *p;
	uint8_t volatile new_crc;
	new_crc = 0;
	for(p = s; *p != '@'; p++) new_crc ^= *p;
	return new_crc;
}
Grazie!
--
Marco / iw2nzm

 (master) a AVR (slave).
A cosa è collegato il CS dell'AVR? 
Che cos'è? Un ATmega16x?

 #define INVERT_UINT(x) (BitReverseTable256[x & 0xFF] << 8) | (BitReverseTable256[(x >> 8) & 0xFF])
Ricordati che il preprocessore fa proprio una sostituzione letterale 
dell'argomento x all'interno del codice della macro.
Se chiami la macro di sopra con:
c = INVERT_UINT(a | b);
Il codice diventa:
c = (BitReverseTable256[a | b & 0xFF] << 8) | BitReverseTable256[(a | b 
 >> 8) & 0xFF])
Che, in entrambi i casi, è la cosa sbagliata perché l'OR tra a e b ha 
meno precedenza dell'AND e dello shift.
Quindi è bene sempre racchiudere gli argomenti tra parentesi in modo da 
non trovarsi brutte sorprese a causa di precedenze inattese tra le 
operazioni:
#define INVERT_UINT(x) (BitReverseTable256[(x) & 0xFF] << 8) | 
BitReverseTable256[((x) >> 8) & 0xFF])
Ciao

 Che cos'č? Un ATmega16x?
Un ATmega644.
Ciao!
Marco / iw2nzm

 BitReverseTable256[((x) >> 8) & 0xFF])
Grazie, di solito lo faccio ma questa volta mi è sfuggita.
Marco / iw2nzm

 Provo a seguire le due strade e faccio sapere.
Ho usato un doppio buffer disabilitando gli interrupt durante la copia 
del primo al secondo:
cli();
for (i = 0; i < SPI_BUF_SIZE; i++) SPIbuf1[i] = INVERT_BYTE(SPIbuf[i]);
sei();
dove SPIbuf1 è dichiarato localmente nella funzione di decodifica.
Il comportamento però non è cambiato di una virgola. A questo punto 
potrebbe essere un problema hardware ma con gli strumenti a mia 
disposizione (oscilloscopio...) mi sembra tutto corretto: polarità 
clock, ss, tempi, ecc...
Marco / iw2nzm

 Mi sono fidato del fatto che impiego meno di 50 ms a calcolarlo :(
E' un fatto che hai accertato? L'hai accertato in tutte le condizioni?
Sai, se ti arrivano un tot di interrupt in quel periodo, si potrebbe
allungare di parecchio.
Se ti avanzano due pin, io ne alzerei uno all'inizio del calcolo del cks
per poi abbassarlo alla fine, mentre alzerei l'altro all'inizio di ogni
interrupt handler per poi abbassarlo alla fine.
Semmai scopri qualcosa di interessante...

 static const unsigned char BitReverseTable256[] =
e cappottare 1 o 2 byte con 17 istruzioni ? troppo tempo ?
saluti

 interrupt handler per poi abbassarlo alla fine.
E' piů o meno quello che ho fatto. Comunque anche disabilitando gli 
interrupt e usando il doppio buffer non ho risolto...
Marco / iw2nzm

 e cappottare 1 o 2 byte con 17 istruzioni ? troppo tempo ?
Mi sembrava più semplice questa soluzione. Dici che potrebbe essere 
sbagliata la tabellina? Non l'ho controllata byte per byte, nel caso lo 
faccio.
Marco / iw2nzm

Forse ho risolto.
Il problema stava qua (idiota io visto che lo so che non si fa)
ISR(TIMER0_COMPA_vect) {
     if (++tick == TICK) {
         tick = 0;
         if (SPItimeout < SPI_TIMEOUT) {
             SPItimeout++;
             update_all();
         } else {
             cold_n_dark();
         }
     }
}
Invece di chiamare dentro l'ISR (sgrunt) update_all, imposto un flag e 
la funzione la chiamo dal main loop. Come del resto anche la SPIDecode...
Per il momento sono spariti gli errori...
Marco / iw2nzm

 sbagliata la tabellina?
non sara' mica fatta a mano ?
saluti

 la funzione la chiamo dal main loop. 
Glub. Perché non s'ha da fare?

 Glub. Perché non s'ha da fare?
Tu mi insegni che le ISR devono essere il piů brevi possibile per 
rilasciare immediatamente il controllo al ciclo principale e agli altri 
interrupt. Ma in questo caso il problema era un altro (credo): 
l'interrupt del timer ha circa lo stesso periodo dell'arrivo dei 
pacchetti, ma ovviamente sono eventi asincroni. Per cui capitava che 
interferivano tra loro dal momento che utilizzavano gli stessi dati.
Comunque, spostando il momento della chiamata alla funzione dalla ISR al 
ciclo principale ho risolto il problema.
Ciao!
Marco / iw2nzm

 pacchetti, ma ovviamente sono eventi asincroni.
Ah, ok, non era una prescrizione generale.
M'avevi fatto prendere un colpo, decenni di certezze svaniti cosě...

 funzione la chiamo dal main loop. Come del resto anche la SPIDecode...
Io l'ho preso per vizio....i miei interrupt sono quasi sempre dei 
PushFifoTask(ProcessID) e i miei main un  switch(PopFifoTask()) con un 
default: SleepMode(); break; , basandomi su una fifo (che in realtà è un 
vettore). In questo modo risolvi quasi qualsiasi problema di concorrenza 
visto che hai sempre accesso esclusivo alle risorse (o quasi) ed è facile 
far stare il processore in sleep per il maggior tempo possibile.
La miglior cosa sarebbe farsi dei semafori o comunque usare piccoli RTOS, ma 
non sempre ne vale la pena! Sicuramente in questo modo si riesce quasi 
sempre a eliminare con estrema facilità qualsiasi ritardo software, per 
esempio, e diventa più facile programmare.
Strano, comunque, che il tuo compilatore non ti abbia fatto almeno un 
warning di rischio rientranza et similia!
Ciao
CG 

 warning di rischio rientranza et similia!
Con il gcc non ne ho mai visti. Ricordo invece che il CCS per i PIC me 
li segnalava.
Ciao!
Marco / iw2nzm