Mi primer código STM32 - por favor critícame

Question

Mi primer código STM32 - por favor critícame

#1 de Scott Seidman (10 votos)
#2 de tcrosley (5 votos)
#3 de Steve Melnikoff (3 votos)
#4 de Christian Bongiorno (1 votos)

7

Acabo de escribir mi primer código en STM32: LED parpadeante. Combina fragmentos de diferentes fuentes; por favor, critícame para que pueda aprender cómo escribir el código correcto y no aprender hábitos estúpidos.

#include "stm32f30x.h"

void SysTick_Handler(void);
void TimingDelay_Decrement(void);
void Delay(__IO uint32_t nTime);

static __IO uint32_t TimingDelay;

int main(void)
{
  RCC->AHBENR  |=  ((1UL << 21) );                  // Enable GPIOE clock??
  GPIOE->MODER |= (1 << 2*8);                       // PIN 9 as output

//GPIOE->BSRR = 1 << 9;                             // LED ON
//GPIOE->BSRR = 1 << 9 << 16;                       // LED OFF
//GPIOE->BRR = 1 << 9;                              // LED OFF

    if (SysTick_Config(SystemCoreClock / 1000))
    { 
    /* Capture error */ 
    while (1);
  }

    while(1)
    {
        GPIOE->BSRR = 1 << 8;   
        Delay(50);
        GPIOE->BRR = 1 << 8;
        Delay(50);
    }
}

void SysTick_Handler(void)
{
  TimingDelay_Decrement();
}

void Delay(__IO uint32_t nTime)
{
  TimingDelay = nTime;

  while(TimingDelay != 0);
}

void TimingDelay_Decrement(void)
{
  if (TimingDelay != 0x00)
  { 
    TimingDelay--;
  }

}

¿Por qué tengo que habilitar el reloj GPIOE? ¿Y cuál es la diferencia entre 1UL << 21 y 1 << 21 ?

c stm32 code cortex-m

pregunta Ekci

4 respuestas

5

En general, se ve muy bien. Podría usar algunos comentarios más.

¿SysTick_Handler es un manejador de interrupciones? Si es así, dilo. Me sorprende que no haya atributos que declaren esto (la mayoría de las C incrustadas para otros procesadores tienen la palabra interrupción en algún lugar i la definición del nombre del controlador). Supongo que debe configurarse en #DEFINE en un archivo .h del sistema en algún lugar. Lástima que lo escondan así.

¿Con qué frecuencia se llama al SysTick_Handler? Sé que está configurado en la llamada SysTick_Config, pero no dices nada al respecto.

¿Cuáles son las unidades para la llamada de retraso? Milisegundos? Dilo.

¿Qué LED de la placa está manipulando con GPIOE- > BSRR = 1 < < 8?

¿Es realmente necesaria la función TimingDelay_Decrement? ¿Por qué no solo poner su código dentro de SysTick_Handler? Por lo general, se intenta evitar llamadas innecesarias dentro de un controlador de interrupciones. No hace una diferencia aquí, pero podría en algún momento manejadores críticos.

respondido por el tcrosley

3

Por motivos de legibilidad, y para reducir la probabilidad de errores, use macros en lugar de números mágicos.

stm32f30x.h contiene macros para (con suerte) todos los valores de registro . Entonces:

RCC->AHBENR  |=  ((1UL << 21) );                  // Enable GPIOE clock??
GPIOE->MODER |= (1 << 2*8);                       // PIN 9 as output

se puede escribir como:

RCC->AHBENR  |= RCC_AHBENR_GPIOEEN;    // Enable GPIOE clock.
GPIOE->MODER |= GPIO_MODER_MODER8_0;   // PIN 8 - not 9! - as output.

(De hecho, el comentario original fue incorrecto aquí. ¡Usar una macro hace que esto sea más obvio!)

Del mismo modo, el código de alternancia de pin podría escribirse como:

GPIOE->BSRR = GPIO_BSRR_BS_8;   
Delay(50);
GPIOE->BRR = GPIO_BRR_BR_8;
Delay(50);

Yendo más lejos, GPIO_MODER_MODER8_0 no está tan claro, así que podría agregar una macro propia:

#define GPIO_MODER_MODER8_OUT   GPIO_MODER_MODER8_0

Podría ser un poco aburrido si estuvieras configurando muchos pines, por lo que podrías hacer:

#define GPIO_MODER_OUT(pin)     (((uint32_t) 1) << (2 * (pin)))

Además, la línea anterior supone que esos bits en MODER ya eran 0x0 o 0x1. Si no podemos asumir eso, primero debemos eliminarlos:

GPIOE->MODER &= ~GPIO_MODER_MODER8;
GPIOE->MODER |= GPIO_MODER_OUT(8);

También:

Si las funciones declaradas en la parte superior del archivo no se utilizan en ningún otro lugar, declare static . Esto evita que se utilicen en cualquier otro archivo por error.

Si el compilador lo permite, main debe declararse como void , no devolver un int , ya que nunca regresa.

Este es un verdadero nitpick: TimingDelay es solo un número regular; no se utiliza cuando una representación hexadecimal es apropiada. Entonces, en TimingDelay_Decrement , donde se compara con cero, use 0 , no 0x00 . Esto también lo hace coherente con la comparación en Delay .

Si he entendido correctamente, __IO marca una variable como volatile , y se sospecha que se usa para marcar registros que son legibles y de escritura. TimingDelay realmente necesita ser volatile , ya que se actualiza en una interrupción, pero me inclino a usar volatile en lugar de __IO , para indicar que no es un registro.

nTime no necesita ser volatile (o __IO ), ya que es solo un parámetro de función normal, pasado por valor, que se lee una vez.

respondido por el Steve Melnikoff

1

Esto técnicamente debería pertenecer a revisión de código .

Mi única recomendación es que los enmascaramientos / cambios de bits deban ser macros o funciones que describan lo que está sucediendo. Los comentarios están bien, pero no funcionan y, en última instancia, lo que digan los comentarios no significa nada; es el código lo que cuenta (y se desincronizarán). Haga que el código sea los comentarios con descomposición funcional:

RCC->AHBENR  |=  ((1UL << 21) );

se convierte en

enableGpioe() {
    RCC->AHBENR  |=  (1UL << 21);  
}

y esto

GPIOE->BSRR = 1 << 9;

se convierte en

void setLEDStatus(LED led, Status status) {
     led = 1 << status;     // I don't know your struct members.
}

cuando

Led

y

Estado

son enumeraciones. Como mínimo, puedes hacer MACROS. Esto evitará que tenga que recordar el enmascaramiento de bits correcto para realizar la tarea.

respondido por el Christian Bongiorno

Lea otras preguntas en las etiquetas c stm32 code cortex-m

Cómo la retroalimentación negativa aumenta el ancho de banda ¿Qué es esta cosa del tubo? (Pedal de batería Madcatz)

score 10 · Accepted Answer

En los STM32s, todos los periféricos están deshabilitados por defecto y no reciben ninguna señal de reloj. Esto ahorra un montón de poder (y te obliga a pensar en las interacciones entre los pines a medida que codificas). Si desea utilizar GPIOE, debe indicarle al STM32 que habilite su reloj.

1UL << 21 desplaza un valor unsigned long de 1 21 bits a la izquierda. 1 << 21 desplaza un número valorado en 1, pero es un tipo predeterminado, que puede o no ser unsigned long , y no necesariamente sabe qué tan amplios son los valores predeterminados de su sistema. Variará según la plataforma y el compilador. De hecho, UL también puede variar según la plataforma y el compilador. Lo mejor es usar las definiciones de biblioteca, que son cosas seguras.

(uint32_t)1 << 21 y (uint64_t)1 << 21 no son ambiguos.

Mi primer código STM32 - por favor critícame

Publicaciones recientes del blog

4 respuestas