arduino / arduino/ArduinoCore-sam

adc_configure_trigger bug

Open
#130 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
HTML
Stars
91
Forks
112
PR merge metrics
No merged PRs in 30d

Description

in `adc.c`

the library code:
```c
/**
* \brief Configure conversion trigger and free run mode.
*
* \param p_adc Pointer to an ADC instance.
* \param trigger Conversion trigger.
* \param uc_freerun ADC_MR_FREERUN_ON enables freerun mode,
* ADC_MR_FREERUN_OFF disables freerun mode.
*
*/
void adc_configure_trigger(Adc *p_adc, const enum adc_trigger_t trigger,
uint8_t uc_freerun)
{
//Warning ADC_MR_TRGSEL_Msk does not include ADC_MR_TRGEN.
p_adc->ADC_MR &= ~(ADC_MR_TRGEN | ADC_MR_TRGSEL_Msk | ADC_MR_FREERUN); //Clear all bits related to triggers and freerun

//Configure FreeRun
if(uc_freerun & ADC_MR_FREERUN == ADC_MR_FREERUN_ON) { //FreeRun is enabled
p_adc->ADC_MR |= ADC_MR_FREERUN_ON;

//Free Run Mode: Never wait for any trigger
//No need to continue and enable hardware triggers
return;
}

//Configure hardware triggers
if(trigger & ADC_MR_TRGEN == ADC_MR_TRGEN_EN) { //Hardware trigger is enabled
p_adc->ADC_MR |= (trigger & ADC_MR_TRGSEL_Msk) | ADC_MR_TRGEN_EN; //Set trigger selection bits and enable hardware trigger
}
}
```

...will not enable frerrun if passed `ADC_ML_FREERUN_ON` (`0x1u<<7`). Because `==` has higher precedence than `&` The function will enable freerun *only* when passed 1. It will not enable freerun when passed `ADC_MR_FREERUN_ON` .

The corrected line can simply be
```c
if(uc_freerun == ADC_MR_FREERUN_ON) ...
```

Which is probably the desired behavior according to the documentation, but may break existing usage. Alternatively

```
if(uc_freerun != ADC_MR_FREERUN_OFF) ...
```

will enable freerun when passing anything other than ADC_FREERUN_OFF.

Contributor guide

No contributing guide indexed for this repository

Research direction

Open adc.c and inspect adc_configure_trigger, focusing on the operator precedence in the uc_freerun condition. Compare the documented ADC_MR_FREERUN_ON and ADC_MR_FREERUN_OFF values with the proposed condition, and check existing callers for compatibility. Done means the documented freerun value enables freerun without changing unrelated trigger behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
embedded-iot
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.