Diferentes implementaciones de una libreria

[Carl47D]

Que tal, como van?

Estoy haciendo los archivos header y source para un integrado que se lee por SPI, el integrado es el TDC1000 de TI y tiene varios registros que se pueden leer y escribir, pero mientras me llega y le hago su PCB para hacer pruebas me puse a testear con la PC.

Mi duda es que implementación será más fácil de usar para un tercero o cual es la manera de hacer librerías que me recomiendan ustedes que seguro tienen mas practica que yo.

Por ejemplo, para el registro CONFIG_0:


En la primer implementación de la librería el header me quedó algo asi:

Código: [Seleccionar]

#ifndef TDC1000_H_
#define TDC1000_H_

/***** Register address *****/

#define TDC1000_CONFIG_0_ADDR   0x00
...

/***** CONFIG_0 Register  *****/
#define TDC1000_CONFIG_0_NUM_TX_MASK 0x1F
#define TDC1000_CONFIG_0_TX_FREQ_DIV_MASK 0x07
#define TDC1000_CONFIG_0_TX_FREQ_DIV_Pos 5
#define TDC1000_CONFIG_0_TX_FREQ_DIV_BY2       0x00 << TDC1000_CONFIG_0_TX_FREQ_DIV_Pos)
#define TDC1000_CONFIG_0_TX_FREQ_DIV_BY4       0x01 << TDC1000_CONFIG_0_TX_FREQ_DIV_Pos)
#define TDC1000_CONFIG_0_TX_FREQ_DIV_BY8       0x02 << TDC1000_CONFIG_0_TX_FREQ_DIV_Pos)
#define TDC1000_CONFIG_0_TX_FREQ_DIV_BY16     0x03 << TDC1000_CONFIG_0_TX_FREQ_DIV_Pos)
#define TDC1000_CONFIG_0_TX_FREQ_DIV_BY32     0x04 << TDC1000_CONFIG_0_TX_FREQ_DIV_Pos)
#define TDC1000_CONFIG_0_TX_FREQ_DIV_BY64     0x05 << TDC1000_CONFIG_0_TX_FREQ_DIV_Pos)
#define TDC1000_CONFIG_0_TX_FREQ_DIV_BY128   0x06 << TDC1000_CONFIG_0_TX_FREQ_DIV_Pos)
#define TDC1000_CONFIG_0_TX_FREQ_DIV_BY256   0x07 << TDC1000_CONFIG_0_TX_FREQ_DIV_Pos)

...
#endif /* TDC1000_H_ */

Y en el main se usaría mas o menos así:

Código: [Seleccionar]

#include "TDC1000.h"

int main(){

    SPI_Write(TDC1000_CONFIG_0_ADDR, TDC1000_CONFIG_0_TX_FREQ_DIV_BY4);

}
Esta implementación no me termina de convencer porque los #define quedaron muy largos xD, además que nada me impide escribirle a los registros valores que puedan llevar al IC a comportarse erróneamente, por ejemplo, escribiendole a bits reservados en los registros.





Asi que inicie una segunda implementación usando typedef enums y typedef structs:

Código: [Seleccionar]

#ifndef TDC1000_H_
#define TDC1000_H_

/***** Register address *****/

#define TDC1000_CONFIG_0_ADDR   0x00

/***** CONFIG_0 Register *****/

typedef enum{
    NUM_TX_1_PULSE,
    NUM_TX_2_PULSES,
    NUM_TX_3_PULSES,
    NUM_TX_4_PULSES,
    NUM_TX_5_PULSES
} NUM_TX_TypeDef;

typedef enum {
    TX_FREQ_DIV_BY2,
    TX_FREQ_DIV_BY4,
    TX_FREQ_DIV_BY8,
    TX_FREQ_DIV_BY16,
    TX_FREQ_DIV_BY32,
    TX_FREQ_DIV_BY64,
    TX_FREQ_DIV_BY128,
    TX_FREQ_DIV_BY256
} TX_FREQ_DIV_TypeDef;

typedef struct {
    NUM_TX_TypeDef NUM_TX : 5;
    TX_FREQ_DIV_TypeDef TX_FREQ_DIV : 3;
} __attribute__((__packed__)) TDC1000_CONFIG_0_TypeDef;

#endif /* TDC1000_H_ */

Y se usaría mas o menos así:

Código: [Seleccionar]

#include <stdio.h>
#include <stdint.h>
#include "TDC1000.h"

#define TDC1000_CONFIG_0_TX_FREQ_DIV_Pos    5  // esto solo lo uso en la pc para crear un "registro falso"

volatile uint8_t CONFIG_0 = 0; // mi registro falso

void TDC1000_setCONFIG_0_Reg(TDC1000_CONFIG_0_TypeDef value);
uint8_t TDC1000_getCONFIG_0_Reg(void);

int main(int argc, char *argv[]){

    TDC1000_CONFIG_0_TypeDef mydef;
    uint8_t test;

    mydef.NUM_TX = NUM_TX_3_PULSES;
    mydef.TX_FREQ_DIV = TX_FREQ_DIV_BY128;

    TDC1000_setCONFIG_0_Reg(mydef);
    test = TDC1000_getCONFIG_0_Reg();

    printf("%x\r\n", test);

    return 0;
}

void TDC1000_setCONFIG_0_Reg(TDC1000_CONFIG_0_TypeDef value){
    // SPI_Write(TDC1000_CONFIG_0_ADDR, value);  // La linea de abajo hace lo mismo que haría esta esta linea ya teniendo el IC
    /* Le escribo al registro CONFIG_0 el valor definido por TDC1000_CONFIG_0_TypeDef, recorro el campo TX_FREQ_DIV porque no puedo hacer un cast directamente de un TDC1000_CONFIG_0_TypeDef a un uint8_t, el compilador se queja :/ */
    CONFIG_0 = (value.TX_FREQ_DIV << TDC1000_CONFIG_0_TX_FREQ_DIV_Pos) |
                value.NUM_TX;
}

uint8_t TDC1000_getCONFIG_0_Reg(void){
    return CONFIG_0;
}
Esta implementación es la que mas me convence, primero porque no quedaron #defines tan largos, segundo a la hora de crear el mydef obtengo "ayuda" extra porque el IDE me da las opciones que puedo escribir en los diferentes campos de los registros, tercero de alguna manera "protejo" las escrituras en los registros, porque ya tengo "controlados" los valores que puedo escribir en los registros, si por ejemplo escribo un TDC1000_CONFIG_2_TypeDef en una función que espera un TDC1000_CONFIG_0_TypeDef me marcará error el compilador.
Además de que es la que creo usaron para hacer las librerías de CMSIS  a la hora de definir los registros de los ARM Cortex.

¿Qué implementación usarian ustedes?
¿Alguna mejora que le pueda hacer a la segunda implementación?

Saludos y de antemano gracias

PD: Soy lo que sigue de malo redactando xD, cualquier duda o cosa que no haya escrito bien háganmelo saber .
PD2: No lo puse en el foro de C para PICs porque creo que no solo aplica para ese MCU, mas bien cualquier cosa que se programe en C.
PD3: el __attribute__ (__packed__) lo uso para forzar que el struct ocupe 8bits (1 byte).

[Carl47D]

Segui trabajando en la segunda implementación y ya puedo hacer un cast de un TDC1000_CONFIG_0_TypeDef a uint8_t, no modifico la pregunta original para no revolverla con una de las soluciones.

Lo solucione con un union, entre el struct que contiene los campos del registro y un uint8_t

Código: [Seleccionar]

#include <stdio.h>
#include <stdint.h>

// CONFIG_0 Register
#define TDC1000_CONFIG_0_TX_FREQ_DIV_Pos    5

typedef enum{
    NUM_TX_1_PULSE,
    NUM_TX_2_PULSES,
    NUM_TX_3_PULSES,
    NUM_TX_4_PULSES,
    NUM_TX_5_PULSES
} NUM_TX_TypeDef;

typedef enum {
    TX_FREQ_DIV_BY2,
    TX_FREQ_DIV_BY4,
    TX_FREQ_DIV_BY8,
    TX_FREQ_DIV_BY16,
    TX_FREQ_DIV_BY32,
    TX_FREQ_DIV_BY64,
    TX_FREQ_DIV_BY128,
    TX_FREQ_DIV_BY256
} TX_FREQ_DIV_TypeDef;

typedef union{
    uint8_t TDC1000_CONFIG_0_Reg;

    struct {
        NUM_TX_TypeDef NUM_TX : 5;
        TX_FREQ_DIV_TypeDef TX_FREQ_DIV : 3;
    } __attribute__((__packed__));

} TDC1000_CONFIG_0_TypeDef;

volatile uint8_t CONFIG_0 = 0;

void TDC1000_setCONFIG_0_Reg(TDC1000_CONFIG_0_TypeDef value);
uint8_t TDC1000_getCONFIG_0_Reg(void);

int main(int argc, char *argv[])
{
    TDC1000_CONFIG_0_TypeDef mydef;
    uint8_t test;

    mydef.NUM_TX = NUM_TX_3_PULSES;
    mydef.TX_FREQ_DIV = TX_FREQ_DIV_BY128;

    printf("%x\r\n", mydef);

    printf("%x\r\n", mydef.TDC1000_CONFIG_0_Reg);

    TDC1000_setCONFIG_0_Reg(mydef);
    test = TDC1000_getCONFIG_0_Reg();

    printf("%x\r\n", test);

    return 0;
}

void TDC1000_setCONFIG_0_Reg(TDC1000_CONFIG_0_TypeDef value){
    CONFIG_0 = value.TDC1000_CONFIG_0_Reg;
}

uint8_t TDC1000_getCONFIG_0_Reg(void){
    return CONFIG_0;
}

A la salida obtengo:

Código: [Seleccionar]

c2
c2
c2
Que si es lo que debería obtener

[KILLERJC]

Yo iria asi:

Código: C

1. #ifndef TDC1000_H_
2. #define TDC1000_H_
3.  
4. /***** Register address *****/
5.  
6. #define TDC1000_ADDR_CONFIG_0   0x00
7. ...
8.  
9. /***** CONFIG_0 Register Mask *****/
10.  
11. #define TDC_PULSE_MASK 0x1F                             // Estas mascaras son para uso interno.. cuando creas una funcion por ejemplo GetFreqDiv();
12. #define TDC_FREQ_DIV_MASK 0x07
13.  
14. /***** CONFIG_0 Register Options *****/
15.  
16. #define TDC_FREQ_DIV_DEFAULT 0x40
17. #define TDC_FREQ_DIV_BY2       0x00
18. #define TDC_FREQ_DIV_BY4       0x20
19. #define TDC_FREQ_DIV_BY8       0x40
20. #define TDC_FREQ_DIV_BY16     0x60
21. #define TDC_FREQ_DIV_BY32     0x80
22. #define TDC_FREQ_DIV_BY64     0xA0
23. #define TDC_FREQ_DIV_BY128   0xC0
24. #define TDC_FREQ_DIV_BY256   0xE0
25.  
26. #define TDC_PULSE_DEFAULT     0x05
27.  
28. ...
29. #endif /* TDC1000_H_ */

Código: C

1. SPI_Write(TDC1000_ADDR_CONFIG_0, TDC_FREQ_DIV_BY4 | TDC_PULSE_DEFAULT);
2. SPI_Write(TDC_ADDR_CONFIG_0, TDC_FREQ_DIV_BY4 | 31);

Simplemente por que sino deberias crear otra funcion que use SPI_Write, y que tenga ciertas precauciones.
En C aquel que use la libreria va a tener que ver si o si el header si quiere realmente hacer algo. un comentario diciendo que son las opciones bastaria.

Tampoco veo razonable crear 32 define con TDC_PULSE_xx por ejemplo. Pero se puede hacer, ya que lo vas a hacer una sola ves nomas.

Si vas a crear una libreria no estarias modificando tampoco 1 solo registro. Sino que modificarias VARIOS con una sola funcion. Y ademas se aseguraria que los valores pasados son correctos.
Por lo cual no te manejarias con SPI_Write, sino por ejemplo con un TDC_Init(FREQ_DIV_128 | NUM_PULSE_5, Aca opciones CONFIG_1, Aca opciones CONFIG_2);
Y ahi si dentro de esa funcion es donde se linkea a las librerias de SPI_Write para poder usarlo.

Los define son cortos, la persona que deba programarlo debera ver el header SI o SI, quiera o no. Y sabra a que seccion le corresponde cada opcion.

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

Personalmente no lo haria con el struct por que es algo que ocupa espacio, si tenes pensado hacer algo asi para cada registro del TDC1000 estarias ocupando bastante memoria dentro del micro, ademas de miles de definiciones de struct, y no recuerdo si lei mal pero no hay forma de asgurarse el orden de los bitfield y es dependiende del compilador.

[Carl47D]

Personalmente no lo haria con el struct por que es algo que ocupa espacio, si tenes pensado hacer algo asi para cada registro del TDC1000 estarias ocupando bastante memoria dentro del micro, ademas de miles de definiciones de struct, y no recuerdo si lei mal pero no hay forma de asgurarse el orden de los bitfield y es dependiende del compilador.

Seguro, son aprox. 15 registros, solo puedo escribirle a 5 o 6, pero aun asi van a usar memoria los TypeDef que le haga a cada uno.

El problema con los define es que en realidad son dos diferentes integrados, un TDC1000 y un TDC7200, asi que si o si tengo que ocupar el nombre completo :l al principio.

Creo que las precauciones serán como dices, las haré como primer paso dentro de las funciones haciendo && entre los valores pasados y los mask definidos en el header.

Gracias por la ayuda, en cuanto tenga las librerías terminadas las paso acá por si tienen cosas por mejorar.
Saludos