Программно-реализованная ШИМ со сдвигом фазы для нагревателя

Предоставленный код управляет 3-фазным нагревателем, генерируя сигнал ШИМ (в программном обеспечении), который управляет 3 симисторами, подключенными к нагревателю. Между MCU (работающим под управлением Zephyr RTOS) и симисторами находится расширитель ввода-вывода PCF8574. Идея состоит в том, чтобы просто сообщить модулю нагревателя, какой процент мощности требуется.

Кроме того, реализован фазовый сдвиг, чтобы уменьшить большие переходы в потребляемом токе. Кроме того, симисторы должны быть по возможности равномерно нагружены, чтобы они одинаково деградировали со временем. Это обрабатывается в PWM_Start().

В коде я добавил // CODE REVIEW COMMENT: чтобы дать некоторый контекст, где это возможно, например, для конкретных вещей Zephyr.

При написании этого кода у меня уже были некоторые сомнения:

  1. Должен ли я использовать функции включения / выключения и запуска / остановки или иметь параметры для этого?
  2. Я использую ASSERTS правильно или я должен просто проверить это в коде?
  3. Должен ли я предотвратить запуск таймеров в граничных случаях 0% и 100% рабочего цикла? Я чувствую, что, может быть, это делает код менее универсальным с небольшой пользой?
  4. Я, вероятно, должен разделить код PWM со сдвигом фазы в специальный файл, но я боюсь, что это потребует вызова функций обратного вызова на краях PWM. Я борюсь с компромиссом, так как код PWM используется только здесь на данный момент.

Примечания

  1. __ASSERT является предоставлено Zephyr.

  2. k_work_init() является:

    static inline void k_work_init(struct k_work *work,  k_work_handler_t handler)
    {
      *work = (struct k_work)Z_WORK_INITIALIZER(handler);
    }
    
  3. k_work_handler_t является:

    typedef void (*k_work_handler_t)(struct k_work *work);
    

Код

Реализация .c файл

/*********************************************************************
 * INCLUDES
 */

#include <zephyr.h>

#include "heater.h"

#include "pcf8574.h"

#include <logging/log.h>
LOG_MODULE_REGISTER(Heater, LOG_LEVEL_DBG);

/*********************************************************************
 * DEFINES
 */

// Time (in seconds) between phase activations
#define TIME_BETWEEN_PHASES   3

#define PWM_PERIOD            (TRIAC_COUNT * TIME_BETWEEN_PHASES)

/*********************************************************************
 * MACROS
 */

/*********************************************************************
 * TYPEDEFS
 */

typedef struct
{
  struct k_timer timer;
  bool           active;
}
S_TriacInfo;

typedef enum
{
  TRIAC_R = 0,
  TRIAC_S,
  TRIAC_T,

  TRIAC_COUNT,
}
E_Triacs;

typedef struct
{
  struct k_work work;
  E_Triacs      triac;
}
S_PWMWork;

/*********************************************************************
 * CONSTANTS
 */

static const E_PCF8574Ports triacToPortMapping[TRIAC_COUNT] = { PCF8574_P4, PCF8574_P5, PCF8574_P6 };

/*********************************************************************
 * PUBLIC VARIABLES
 */
 
/*********************************************************************
 * LOCAL VARIABLES
 */

static S_TriacInfo triacs[TRIAC_COUNT];
static uint8_t     dutyCycle;

static S_PWMWork   flipPWM;

/*********************************************************************
 * LOCAL FUNCTION PROTOTYPES
 */

static void TriacTimerFxn (struct k_timer *timer);

static void FlipPWM (struct k_work *item);

static bool PWM_Start (void);
static bool PWM_StartHighCycle (E_Triacs triac);
static bool PWM_StartLowCycle  (E_Triacs triac);

static bool TurnOnTriac  (E_Triacs triac);
static bool TurnOffTriac (E_Triacs triac);

/*********************************************************************
 * LOCAL FUNCTIONS
 */

static void TriacTimerFxn (struct k_timer *timer)
{
  for (E_Triacs triac = TRIAC_R; triac < ARRAY_SIZE(triacs); triac++)
  {
    if (timer == &triacs[triac].timer)
    {
      flipPWM.triac = triac;
      k_work_submit(&flipPWM.work);
      break;
    }
  }
}

static void FlipPWM (struct k_work *item) // CODE REVIEW COMMENT: This function follows a library typedef, it's always a void
{
  S_PWMWork *pPWMWork = CONTAINER_OF(item, S_PWMWork, work);
  E_Triacs triac = pPWMWork->triac;

  if (triacs[triac].active)
    PWM_StartLowCycle(triac);
  else
    PWM_StartHighCycle(triac);
}

static bool PWM_Start (void)
{
  static E_Triacs currentFirstTriac = TRIAC_R;

  // Start PWM cycle for the first triac
  if (!PWM_StartHighCycle(currentFirstTriac))
  {
    LOG_ERR("Failed to start PWM");
    return false;
  }

  // Start timers to support phase-shifting for the other triacs
  for (uint8_t i = 1; i < ARRAY_SIZE(triacs); i++)
  {
    E_Triacs triac = (currentFirstTriac + i) % ARRAY_SIZE(triacs);

    if (!TurnOffTriac(triac))
    {
      LOG_ERR("Failed to start PWM");
      return false;
    }

    uint16_t timer_ms = 1000 * (triac * TIME_BETWEEN_PHASES);
    k_timer_start(&triacs[triac].timer, K_MSEC(timer_ms), K_SECONDS(0));
  }

  currentFirstTriac = (currentFirstTriac + 1) % ARRAY_SIZE(triacs);

  LOG_DBG("PWM started");

  return true;
}

static bool PWM_StartHighCycle (E_Triacs triac)
{
  __ASSERT(triac >= TRIAC_R &&
           triac <  ARRAY_SIZE(triacs),
           "Invalid triac");

  if (!TurnOnTriac(triac))
  {
    LOG_ERR("Failed to start high cycle for triac %u", triac);
    return false;
  }

  uint16_t timer_ms = 1000 * PWM_PERIOD * dutyCycle / 100;
  k_timer_start(&triacs[triac].timer, K_MSEC(timer_ms), K_SECONDS(0));

  LOG_DBG("Started high cycle for triac %u (%u ms)", triac, timer_ms);

  return true;
}

static bool PWM_StartLowCycle (E_Triacs triac)
{
  __ASSERT(triac >= TRIAC_R &&
           triac <  ARRAY_SIZE(triacs),
           "Invalid triac");

  if (!TurnOffTriac(triac))
  {
    LOG_ERR("Failed to start low cycle for triac %u", triac);
    return false;
  }

  uint16_t timer_ms = 1000 * PWM_PERIOD * (100 - dutyCycle) / 100;
  k_timer_start(&triacs[triac].timer, K_MSEC(timer_ms), K_SECONDS(0));

  LOG_DBG("Started low cycle for triac %u (%u ms)", triac, timer_ms);

  return true;
}

static bool TurnOnTriac (E_Triacs triac)
{
  __ASSERT(triac >= TRIAC_R &&
           triac <  ARRAY_SIZE(triacs),
           "Invalid triac");

  E_PCF8574Ports port = triacToPortMapping[triac];
  if (!PCF8574_ConfigurePin(port, PCF8574_OUTPUT_LOW))
  {
    LOG_ERR("Failed to turn on triac %u", triac);
    return false;
  }

  LOG_DBG("Turned on triac: %u", triac);
  triacs[triac].active = true;

  return true;
}

static bool TurnOffTriac (E_Triacs triac)
{
  __ASSERT(triac >= TRIAC_R &&
           triac <  ARRAY_SIZE(triacs),
           "Invalid triac");

  E_PCF8574Ports port = triacToPortMapping[triac];
  if (!PCF8574_ConfigurePin(port, PCF8574_OUTPUT_HIGH))
  {
    LOG_ERR("Failed to turn off triac %u", triac);
    return false;
  }

  LOG_DBG("Turned off triac: %u", triac);
  triacs[triac].active = false;

  return true;
}

/*********************************************************************
 * PUBLIC FUNCTIONS
 */

bool Heater_Init (void) // CODE REVIEW COMMENT: This is a bool so it follows the style of all other modules, currently there are no fault conditions
{
  LOG_DBG("Initializing heater");

  for (E_Triacs triac = TRIAC_R; triac < ARRAY_SIZE(triacs); triac++)
  {
    k_timer_init(&triacs[triac].timer, TriacTimerFxn, NULL);
    triacs[triac].active = false;
  }

  k_work_init(&flipPWM.work, FlipPWM);

  return true;
}

bool Heater_SetPower (uint8_t powerPercentage)
{
  __ASSERT(powerPercentage <= 100, "Invalid power percentage");

  LOG_DBG("Setting heater power: %u%%", powerPercentage);

  dutyCycle = powerPercentage;

  return PWM_Start();
}

Заголовочный файл .h

#ifndef HEATER_H
#define HEATER_H

/*********************************************************************
 * INCLUDES
 */

#include <stdint.h>

/*********************************************************************
 * DEFINES
 */

/*********************************************************************
 * MACROS
 */

/*********************************************************************
 * TYPEDEFS
 */

/*********************************************************************
 * CONSTANTS
 */

/*********************************************************************
 * PUBLIC VARIABLES
 */

/*********************************************************************
 * PUBLIC FUNCTIONS
 */

bool Heater_Init (void);

bool Heater_SetPower (uint8_t powerPercentage);

#endif // HEATER_H

1 ответ
1

Рассмотрите возможность использования Doxygen

Я вижу, у вас есть какой-то структурированный способ документирования кода с помощью комментариев. Однако я рекомендую вам использовать Doxygenформат для документирования вашего кода. Это гарантирует, что у вас по-прежнему есть удобочитаемые комментарии, документирующие ваш код, но он также поставляется с инструментами, которые могут создавать руководства из комментариев в вашем исходном коде, а также проверять, не забыли ли вы задокументировать все функции, параметры и переменные.

Избегайте макросов для констант

Вместо того, чтобы использовать #define для констант объявите static const переменные. Преимущество в том, что они будут иметь правильный тип, и вы избежите потенциальных проблем, если у вас есть epxressions, но вы забыли добавить круглые скобки (хотя вы сделали это правильно в своем коде). Так например:

static const uint16_t TIME_BETWEEN_PHASES = 3;
static const uint16_t PWM_PERIOD = TRIAC_COUNT * TIME_BETWEEN_PHASES;

Не злоупотребляйте enums

Пока enums обычно удобны в использовании, но в некоторых случаях они могут быть проблематичными. Например, в этой строке:

for (E_Triacs triac = TRIAC_R; triac < ARRAY_SIZE(triacs); triac++)

Ты уверен что TRIAC_R это первый симистор? Что, если кто-то изменит порядок декларации E_Triacs? Поскольку вы просто хотите перебрать все элементы triacs, Я бы не стал использовать enum для итератора цикла здесь, а скорее обычное целое число, и начинаются с 0 явно:

for (uint16_t triac = 0; triac < ARRAY_SIZE(triacs); triac++)

Глядя на остальную часть кода, TRIAC_S и TRIAC_T никогда не используются, и даже TRIAC_COUNT не используется много. Это говорит о том, что вам не очень интересно давать каждому симистору символическое имя, это всего лишь три симистора. Поэтому я бы просто использовал обычное целое число:

enum {
    TRIAC_COUNT = 3, // So we can still use it to declare arrays
};

typedef struct
{
  struct k_work work;
  uint16_t      triac;
}
S_PWMWork;

Это также упрощает некоторые утверждения, если вы сохраняете номер симистора в беззнаковом int, например:

__ASSERT(triac >= TRIAC_R && triac <  ARRAY_SIZE(triacs), "Invalid triac");

Становится:

__ASSERT(triac <  ARRAY_SIZE(triacs), "Invalid triac");

В любом случае это утверждение было немного странным, поскольку вы уже предположили, что триаки были пронумерованы, начиная с 0 в таких заявлениях:

E_Triacs triac = (currentFirstTriac + i) % ARRAY_SIZE(triacs);

О проверке ошибок

В вашем коде происходит много проверок ошибок. Давайте посмотрим на:

if (!PCF8574_ConfigurePin(port, PCF8574_OUTPUT_LOW))

Меня это удивляет; может ли установка порта на низкий или высокий уровень когда-либо выйти из строя на вашем MCU? И возвращаемое значение PWM_StartLowCycle() и PWM_StartHighCycle() игнорируются в FlipPWM() так или иначе. Какой смысл проверять ошибки, если вы ничего не делаете с ошибкой?

Либо он никогда не может выйти из строя во время работы (хотя, возможно, вы могли бы провести простой однократный тест в начале своей программы, чтобы убедиться, что контакты и порты настроены правильно), и в этом случае я бы не стал выполнять эти проверки ошибок, либо он может сбой, и в этом случае я бы добавил код для разрешения ситуации. Подумайте, что может пойти не так, если симистор застрянет во включенном или выключенном положении. Мог ли он перегреться? Если он перестанет нагреваться, может что-нибудь замерзнуть? Есть ли способ получить сигнал тревоги, чтобы уведомить человека о вмешательстве? Есть ли безопасный режим, в который можно было бы перейти, пока проблема не исчезнет?

Рассмотрите возможность объединения функций запуска / остановки в одну

Действительно, вы думали о том, следует ли иметь отдельные функции запуска и остановки или одну функцию, которая принимает параметр. Учтите, что большая часть кода в PWM_StartHighCycle() и PWM_StartLowCycle() то же самое, поэтому есть много дублирования, которое можно удалить. То же самое касается TurnOnTriac() и TurnOffTriac(). Поэтому я бы действительно попытался объединить эти функции, чтобы они выглядели так:

static void PWM_StartCycle(uint16_t triac, bool high) {
    ...
    SetTriac(triac, high);
    ...
}

static void SetTriac(uint16_t triac, bool on) {
    ...
}

Это также упростило бы FlipPWN():

static void FlipPWM (struct k_work *item) {
  S_PWMWork *pPWMWork = CONTAINER_OF(item, S_PWMWork, work);
  E_Triacs triac = pPWMWork->triac;
  PWM_StartCycle(triac, !trias[triac].active);
}

Обработка пограничных случаев

Если рабочий цикл 0% и 100% являются допустимыми рабочими точками, я бы позаботился о том, чтобы они обрабатывались правильно. Если вы не сделаете их особенным, произойдет то, что вы все равно будете переключаться между включением / выключением, но, например, при 0% у вас все еще будет очень небольшое время, когда он включен. Может ли это быть проблемой? Износит ли это симистор или нагреватель? Было бы относительно просто проверить эту ситуацию и избежать дополнительной нагрузки на систему.

  • 1

    При изменении макросов на константы рекомендуется изменить имена, чтобы можно было использовать только макросы. ALL_CAPS. Это помогает нам поддерживать понимание того, какие идентификаторы не подчиняются обычным правилам области видимости.

    — Тоби Спейт

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *