软件实现加热器的移相脉宽调制

Question

所提供的代码通过产生PWM信号(在软件中)来控制三相加热器，该信号控制连接到加热器的3个三极管。在单片机(运行Zephyr )和triacs之间是一个PCF8574 IO扩展程序。其想法是简单地告诉加热器模块，哪个百分比的功率是需要的。

此外，还实现了相移，以减少大的过渡电流.而且，三合会的负荷应该尽可能均匀，这样随着时间的推移，它们也会同样地退化。这是在PWM_Start()中处理的。

在代码中，我添加了// CODE REVIEW COMMENT:，以给出一些可能相关的上下文，比如特定于Zephyr的东西。

在编写这段代码时，我已经有了一些疑问：

1. 我应该使用on/off和启动/停止函数，还是为此设置参数？
2. 我是否正确地使用了ASSERTS，还是应该在代码中检查它？
3. 我是否应该防止定时器在0%和100%的边界情况下运行？我觉得也许这只会让代码变得不那么通用，只会带来很小的好处？
4. 我可能应该将移相PWM代码拆分为一个专用文件，但恐怕这需要在PWM边缘调用回调函数。我正在挣扎于权衡，因为PWM代码现在只在这里使用。

Notes

1. __ASSERT是由Zephyr。
2. k_work_init() 是：静态内联k_work_init(struct k_work *work，k_work_handler_t处理程序){ *work = (struct K_work)Z_WORK_INITIALIZER(处理程序)；}
3. 胡枝子( k_work_handler_t 是) (*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

Answer 1

考虑使用Doxygen

我看到您有一些使用注释记录代码的结构化方法。然而，我建议您使用含氧的S格式来记录您的代码，这可以确保您仍然有人类可读的注释记录您的代码，但它也提供了一些工具，可以根据源代码中的注释生成手册，并验证您没有忘记记录所有函数、参数和变量。

避免常量

的宏

不要对常量使用#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开头的triacs在如下语句中编号：

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

关于检错

在您的代码中有很多错误检查。让我们看看：

if (!PCF8574_ConfigurePin(port, PCF8574_OUTPUT_LOW))

我对此感到惊讶:在你的单片机上设置一个端口低或高的端口会失败吗？PWM_StartLowCycle()和PWM_StartHighCycle()的返回值在FlipPWM()中也被忽略了。如果您没有对错误做任何操作，那么检查错误的意义是什么？

要么在运行时它永远不会失败(也许您可以在程序开始时做一个简单的一次性测试，以确保正确配置引脚和端口)，在这种情况下，我不会执行这些错误检查，或者失败，在这种情况下，我会添加一些代码来解决这种情况。想一想，如果一个triac卡在on或off位置上，会出现什么问题。它会过热吗？如果它停止加热，什么东西会结冰吗？有没有办法发出警报信号通知人类干预？在问题持续的时候，你能进入一个安全的模式吗？

考虑将开始/停止函数合并到一个

中

实际上，您在考虑是否应该有单独的开始函数和停止函数，而不是一个接受参数的函数。考虑到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%，你将仍然有一个非常小的时间，它是开。这会是个问题吗？那会耗尽三合一机还是暖气？对这种情况进行检查并避免对系统造成额外压力是相对简单的。