Optimisation appel de fonctions "composants" en C

[ATdevice]

Bonjour,

Le sujet de mon post concerne l'optimisation d'un programme (ou plutôt bibliothèque de composant) qui fonctionne, mais pour lequel je m'interroge sur une meilleure façon de coder.
En effet j'essaie de reprendre certains de nos programmes afin de mieux dissocier la couche fonctionnelle (haut niveau) de la couche bas niveau (matérielle), pour que d'une part ce soit plus clair pour celui qui repassera derrière, et d'autre part pour que ce soit plus structuré et donc plus facile à faire évoluer.
Je travaille actuellement avec un microcontrôleur de la famille PIC24E, et l'environnement MPLABX (v5.20) et XC16 (v1.41).

Ma situation est plus précisément ce cas de figure :
J'ai une carte principale avec un microcontrôleur (PIC24E) et deux cartes esclaves (chacune avec des fonctions différentes) mais sur lesquelles on retrouve le même convertisseur numérique/analogique AD5660.
Les deux convertisseurs utilisent le même périphérique SPI, et bien sur deux Chip Select différents.

Actuellement l'appel des fonctions se fait de la façon "AD5660_1" et "AD5660_2", le problème est que d'un point de vue code, ça oblige à modifier la bibliothèque du composant AD5660, alors que dans l'idéal j'aimerais que celle-ci soit générique, peut importe le nombre de composants, et peu importe si ils utilisent le même périphérique SPI ou non.

Code actuel :

/* AD5660_1_CS_low - Set Chip Select (CS) low.
 * parameter : void.
 * return : void.
 */
void AD5660_1_CS_low(void)
{
    AD5660_1_CS=0;
}

/* AD5660_1_CS_high - Set Chip Select (CS) high.
 * parameter : void.
 * return : void.
 */
void AD5660_1_CS_high(void)
{
    AD5660_1_CS=1;
}

/* AD5660_2_CS_low - Set Chip Select (CS) low.
 * parameter : void.
 * return : void.
 */
void AD5660_2_CS_low(void)
{
    AD5660_2_CS=0;
}

/* AD5660_2_CS_high - Set Chip Select (CS) high.
 * parameter : void.
 * return : void.
 */
void AD5660_2_CS_high(void)
{
    AD5660_2_CS=1;
}

/* AD5660_1_configureHardware - Configure CS pin and SPI peripheral.
 * parameter : void.
 * return : void.
 */
void AD5660_1_configureHardware(void)
{
    AD5660_1_CS_TRIS=OUTPUT;		// Configure Chip Select pin as output
    AD5660_1_CS_high();			// Initialize Chip Select at '1' : un-select device
    SPI1_configure(MASTER,NO_BUFFER,MODE_8_BIT,AD5660_PPRE,AD5660_SPRE,AD5660_CKE,AD5660_CKP,AD5660_SMP);
}

/* AD5660_2_configureHardware - Configure CS pin and SPI peripheral.
 * parameter : void.
 * return : void.
 */
void AD5660_2_configureHardware(void)
{
    AD5660_2_CS_TRIS=OUTPUT;		// Configure Chip Select pin as output
    AD5660_2_CS_high();			// Initialize Chip Select at '1' : un-select device
    SPI1_configure(MASTER,NO_BUFFER,MODE_8_BIT,AD5660_PPRE,AD5660_SPRE,AD5660_CKE,AD5660_CKP,AD5660_SMP);
}

/* AD5660_1_sendByte - Send data byte to AD5660 1 through SPI.
 * parameter wByte : Data to send.
 * return status : 0 = success; negative = fail.
 */
int AD5660_1_sendByte(unsigned char wByte)
{
	int status=0;
    SPI1_configure(MASTER,NO_BUFFER,MODE_8_BIT,AD5660_PPRE,AD5660_SPRE,AD5660_CKE,AD5660_CKP,AD5660_SMP);
	AD5660_1_CS_low();
    SPI1_transferData((unsigned short)wByte);
	AD5660_1_CS_high();
    return status;
}

/* AD5660_2_sendByte - Send data byte to AD5660 2 through SPI.
 * parameter wByte : Data to send.
 * return status : 0 = success; negative = fail.
 */
int AD5660_2_sendByte(unsigned char wByte)
{
	int status=0;
    SPI1_configure(MASTER,NO_BUFFER,MODE_8_BIT,AD5660_PPRE,AD5660_SPRE,AD5660_CKE,AD5660_CKP,AD5660_SMP);
	AD5660_2_CS_low();
    SPI1_transferData((unsigned short)wByte);
	AD5660_2_CS_high();
    return status;
}

Structure de code souhaitée :

--- HardwareProfile.h ---

// - AD5660 (1)
#define AD5660_1_CS_TRIS        _TRISG12
#define AD5660_1_CS             _LATG12
// - AD5660 (2)
#define AD5660_2_CS_TRIS        _TRISD6
#define AD5660_2_CS             _LATD6

--- AD5660.h ---

#define AD5660_NUM		2

typedef struct
{
	(void*)CS_low(void);
	(void*)configureHardware(void);
	(int*)sendByte(unsigned char wByte);
} _AD5660_struct_;

extern _AD5660_struct_	AD5660_device[AD5660_NUM];

--- AD5660.c ---
#include "HardwareProfile.h"
#include "AD5660.h"
#include "spi1.h"

_AD5660_struct_	AD5660_device[AD5660_NUM];

void AD5660_CS_low(char AD5660_num)
{
	AD5660_device[AD5660_num].CS_low();
}

void AD5660_configureHardware(char AD5660_num)
{
	AD5660_device[AD5660_num].configureHardware();
}

int AD5660_sendByte(char AD5660_num,char data)
{
	return (AD5660_device[AD5660_num].sendByte(data))
}

Une idée de comment faire? Merci
-----

[Vincent PETIT]

Salut,
Ce que tu peux faire c'est mettre en paramètre d'une fonction la configuration du AD5660 alors que là, tu appelles des fonctions où tu as des instructions en dures. Je veux dire que ta configuration est en dure.

ps : je trouve que tu as beaucoup d'appel de fonction inutile mais c'est lié a ce qui est écrit au dessus :

void AD5660_2_CS_high(void)
{
    AD5660_2_CS=1;
}

Ceci va soit être optimisé par le compilateur pour finir directement dans la fonction appelante soit, malheureusement, tu vas faire appel à la pile pour pas grand chose (donc perte de temps)

[activmaker]

Bonjour,

Ceci va soit être optimisé par le compilateur pour finir directement dans la fonction appelante soit, malheureusement, tu vas faire appel à la pile pour pas grand chose (donc perte de temps)

J'utilise des mcu ARM , mais pour la partie optimisation j'ai déjà eu des mauvaises surprises. C'était systématiquement un appel sur la pile (Avec sauvegarde des registres , contexte etc).. Avec les Pic24 , je ne sais pas trop, mais c'est un point à vérifier.

L'ajout d'un directive inline à drastiquement amélioré les perfs (Ce que je voulais) et augmenté la taille du binaire...A utiliser avec parcimonie donc.

[ATdevice]

Ce que tu peux faire c'est mettre en paramètre d'une fonction la configuration du AD5660

Je ne saisis pas l'idée, aurais-tu un exemple?

ps : je trouve que tu as beaucoup d'appel de fonction inutile mais c'est lié a ce qui est écrit au dessus

Oui parce que j'ai des fonctions "HAL" où je passe du périphérique à la fonction utile pour le composant. Par exemple la pin Chip Select, ou l'appel de fonctions relative à SPI1.
Dans la suite de mon programme on ne trouve plus de références aux périphériques, mais seulement aux fonctions "HAL", ce qui fait que si je décide de passer non plus par SPI1 mais par SPI2, j'ai qu'une seule fonction à changer.

Un exemple plus parlant :

/*** AD5660 hardware abstraction layer (HAL) function declarations ***/

/* AD5660_1_sendByte - Send data byte to AD5660 through SPI.
 * parameter wByte : Data to send.
 * return status : 0 = success; negative = fail.
 */
int AD5660_1_sendByte(unsigned char wByte)
{
	int status=0;
	SPI1_configure(MASTER,NO_BUFFER,MODE_8_BIT,AD5660_PPRE,AD5660_SPRE,AD5660_CKE,AD5660_CKP,AD5660_SMP);
	SPI1_transferData((unsigned short)wByte);
	return status;
}

/* AD5660_1_sendWord - Send data word to AD5660 through SPI.
 * parameter wWord : Data to send.
 * return status : 0 = success; negative = fail.
 */
int AD5660_1_sendWord(unsigned short wWord)
{
	int status=0;
	SPI1_configure(MASTER,NO_BUFFER,MODE_8_BIT,AD5660_PPRE,AD5660_SPRE,AD5660_CKE,AD5660_CKP,AD5660_SMP);
	SPI1_transferData((unsigned char)((wWord>>8)&0xFF));
	SPI1_transferData((unsigned char)(wWord&0xFF));
	return status;
}

/*** AD5660 generic function declarations ***/

/* AD5660_1_setDAC - Set AD5660 power mode and DAC value.
 * parameter powerMode : Power mode to set.
 * parameter value : 16-bit value to set.
 * return status : 0 = success; negative = fail.
 */
int AD5660_1_setDAC(unsigned char powerMode,unsigned short value)
{
	int status=0;
	AD5660_1_configureHardware();
	// Send register configuration to AD5660
	AD5660_1_CS_low();
	status|=AD5660_1_sendByte(powerMode);
	status|=AD5660_1_sendWord(value);
	AD5660_1_CS_high();
	return status;
}

/* AD5660_1_setVoltage - Set voltage to AD5660 DAC and update value.
 * parameter powerMode : Power mode to set.
 * parameter vref : DAC reference voltage (decimal value).
 * parameter voltage : Voltage output (decimal value).
 * return status : 0 = success; negative = fail.
 */
int AD5660_1_setVoltage(unsigned char powerMode,double vref,double voltage)
{
	int status=0;
	unsigned short value=0;
	if(vref<=0||vref>AD5660_INT_VREF)
		return -1;
	if(voltage>(vref*2))
		return -1;
	value=(unsigned short)((voltage*65535.0)/(2*vref));
	status=AD5660_1_setDAC(powerMode,value);
	return status;
}

Ce que j'aimerais dans l'idéal c'est avoir ma bibliothèque AD5660.c/AD5660.h avec des fonctions déclarées dans une structure ou autre, sans être dépendant du nombre de composants (j'ai le même cas de figure avec un autre projet où j'ai 5 MCP23S17 à gérer).
Pour exagérer je vais partir sur l'exemple où j'aurais 5 composants identiques, et deux SPI différents :
- AD5660 n°1 et n°2 sur SPI1
- AD5660 n°2 à n°5 sur SPI2

#include "AD5660.h"
#include "HardwareProfile.h"
#include "spi1.h"
#include "spi2.h"

#define AD5660_NUM	5 // use 5 AD5660

typedef struct
{
	(void*)CS_low(void);
	(int*)sendByte(unsigned char wByte);
} _AD5660_struct_;

_AD5660_struct_				AD5660[AD5660_NUM]; 	// Declare AD5660[0] to AD5660[4] function structure

// AD5660 n°1 on SPI1
(void*)AD5660[0].CS_low(void)		GPIO_CS_1_PIN=0;	// Route Chip Select to function
(void*)AD5660[0].CS_high(void)		GPIO_CS_1_PIN=1;	// Route Chip Select to function
(void*)AD5660[0].sendByte(char data)	SPI1_sendByte(data);	// Route SPI peripheral to function

// AD5660 n°2 on SPI1
(void*)AD5660[1].CS_low(void)		GPIO_CS_2_PIN=0;	// Route Chip Select to function
(void*)AD5660[1].CS_high(void)		GPIO_CS_2_PIN=1;	// Route Chip Select to function
(void*)AD5660[1].sendByte(char data)	SPI1_sendByte(data);	// Route SPI peripheral to function

// AD5660 n°3 on SPI2
(void*)AD5660[2].CS_low(void)		GPIO_CS_3_PIN=0;	// Route Chip Select to function
(void*)AD5660[2].CS_high(void)		GPIO_CS_3_PIN=1;	// Route Chip Select to function
(void*)AD5660[2].sendByte(char data)	SPI2_sendByte(data);	// Route SPI peripheral to function

// AD5660 n°4 on SPI2
(void*)AD5660[3].CS_low(void)		GPIO_CS_4_PIN=0;	// Route Chip Select to function
(void*)AD5660[3].CS_high(void)		GPIO_CS_4_PIN=1;	// Route Chip Select to function
(void*)AD5660[3].sendByte(char data)	SPI2_sendByte(data);	// Route SPI peripheral to function

// AD5660 n°5 on SPI2
(void*)AD5660[4].CS_low(void)		GPIO_CS_5_PIN=0;	// Route Chip Select to function
(void*)AD5660[4].CS_high(void)		GPIO_CS_5_PIN=1;	// Route Chip Select to function
(void*)AD5660[4].sendByte(char data)	SPI2_sendByte(data);	// Route SPI peripheral to function

Enfin voilà l'idée de base, je recherche un moyen simple de faire ça ^^ Il me semble qu'en C++ c'est un peu ce qui se passe, on déclare une classe ADC avec les différentes fonctions, etc (je ne suis pas très familier avec le C++ donc je ne sais pas ce qui se cache derrière).

[A voir en vidéo sur Futura]

[ATdevice]

Et j'ai oublié une fonction générique pour l'exemple, où l'on constate qu'on peut appeler "autant" de composants que l'on veut :

/* AD5660_writeData- Send data byte to pointed AD5660 through SPI.
 * parameter dacNum : DAC to address.
 * parameter inData : Data to send.
 * return status : 0 = success; negative = fail.
 */
int AD5660_writeData(char dacNum,char inData)
{
	int fctStatus=0;
	AD5660[dacNum].CS_low();
	fctStatus=AD5660[4dacNum].sendByte(inData);
	AD5660[dacNum].CS_high();
	return fctStatus;
}

[Vincent PETIT]

Justement tu donnes très bon exemple avec les périphériques SPI identiques (les AD5660)
Quand bien même tu en aurais 200, tu n'as absolument pas besoin de 200 objets ou tableau de AD5660. La seule chose qu'il te faut c'est 200 objets/tableau de Chip Select.

Je ne sais pas si tu vois ce que je veux dire ?

int AD5660_writeData(char dacNum,char inData, unsigned short SPI, unsigned short CS)
{
...
}

[ATdevice]

int AD5660_writeData(char dacNum,char inData, unsigned short SPI, unsigned short CS)
{
...
}

Je situe l'idée, comme les modules SPI sont des ensembles de fonctions, et les sorties CS font référence à des registres de type LATxbits.LATxy (par exemple LATFbits.LATF0). il me faudrait quand même des structures pour les fonctions SPI_config(), SPI_write(), CS_low() et CS_high(), que je déclare en tant que tableau pour y accéder avec les paramètres SPI et CS de la fonction AD5660_writeData().

J'aurais donc SPI[n].config() et SPI[n].write(), et CS[m].low() et CS[m].high()
Je n'aurais plus besoin du paramètre dacNum puisque la sélection du composant serait régie par SPI et CS. ou alors éventuellement les remplacer par une macro.

résultat :

int AD5660_writeData(char inData, unsigned short spiPeriph, unsigned short chipSel)
{
   SPI[spiModule].config(pol,baud,idl);  // config SPI baudrate, polartity, idle state, etc.
   CS[chipSel].low();                    // addressed chip CS low
   SPI[spiPeriph].write(inData);         // write byte through addressed SPI peripheral
   CS[chipSel].high();                   // addressed chip CS high
}

Il me reste à vérifier un truc que je n'ai jamais fait, faire une structure pointant sur des fonctions

[Vincent PETIT]

Ca me parait bien et ça réduira le code.
Pour la structure de pointeurs de fonctions, certaines couches HAL sont construites comme ça (celle de ST il me semble)