Skip to content

Bug in SetCCRRegister() #531

Closed
Closed
@cdrose

Description

@cdrose

I have been doing some work with CCP timer outputs and there are a few bugs to fix.

In timer.c attachIntHandleOC() case1 for compare channel 1 is correct, but case 2, 3 and 4 have != HAL_OK when the logic should be == HAL_OK, which then causes the call to HAL_TIM_OC_Start_IT() to be skipped and the channel is not enabled.

eg.

case 2:
      obj->irqHandleOC_CH2 = irqHandle;
      if (HAL_TIM_OC_ConfigChannel(handle, &sConfig, TIM_CHANNEL_2) == HAL_OK) {
        HAL_TIM_OC_Start_IT(handle, TIM_CHANNEL_2);
      }

There is also a bug in the SetCCRRegister() function in timer.c, the __HAL_TIM_SET_COMPARE is meant to be passed the TIM_CHANNEL_X constants (0x00, 0x04, 0x08, 0x0C) but the conversion from integer 1-4 to these values is incorrect. Just multiplying by 4 results in an off by 1 error (either that or we should pass in integer 0-3 but thats not consistent with other usage in timer.c

I suggest a switch instead to make the intent more clear and avoid the odd conversion function:

void setCCRRegister(stimer_t *obj, uint32_t channel, uint32_t value)
{
  switch (channel)
  {
	  case 1: channel = TIM_CHANNEL_1; break;
	  case 2: channel = TIM_CHANNEL_2; break;
	  case 3: channel = TIM_CHANNEL_3; break;
	  case 4: channel = TIM_CHANNEL_4; break;
  }

  __HAL_TIM_SET_COMPARE(&(obj->handle), channel, value);
}

I would also assume this fix needs to be applied to getCCRRegister() too though I havent tested that.

Metadata

Metadata

Assignees

Labels

bug 🐛Something isn't working

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions