Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adc: adc_read() not check for error (#1575) v2 #1599

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ruehlchris
Copy link

Implementation of adc_read() not check for errors in the ADC_CS register.

  • change the return type from uint16_t to int32_t
  • check for ADC_CS_READY_BITS and error ADC_CS_ERR_BITS or ADC_CS_ERR_STICKY_BITS
  • return the ADC value (12bits) or the negative value of ADC_CS_ERR_BITS or ADC_CS_ERR_STICKY_BITS depending on the error

v2:

  • update the code, optimize the workflow
  • no reset required , so removed it.

Implementation of adc_read() not check for errors in the ADC_CS register.
    
-  change the return type from uint16_t to int32_t
-  check for ADC_CS_READY_BITS and error ADC_CS_ERR_BITS or ADC_CS_ERR_STICKY_BITS    
- return the ADC value (12bits) or the negative value of
ADC_CS_ERR_BITS or ADC_CS_ERR_STICKY_BITS depending on the error

return (uint16_t) adc_hw->result;
if ((adc_hw->cs & ADC_CS_READY_BITS)>0) {
return adc_hw->result & 0xfff;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to use ADC_RESULT_BITS

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will have a look if that will have benefit. The original code check for the ADC_CS_READY_BITS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, :)
Thanks for the eye opener.

return adc_hw->result & 0xfff;
}
if ((adc_hw->cs & ADC_CS_ERR_BITS)>0) {
return -ADC_CS_ERR_BITS;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit random to return -ADC_CS_ERR_BITS. It would be better to return a "standard" error code? Maybe PICO_ERROR_GENERIC or PICO_ERROR_NO_DATA?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point, but if we doing this we can't distinguished what caused the error.

Use
PICO_ERROR_NO_DATA
and additional, define a new code:
PICO_ERROR_ADC_STICKY_BIT

}
/* clear sticky bit */
hw_set_bits(&adc_hw->cs, ADC_CS_ERR_STICKY_BITS);
return -ADC_CS_ERR_STICKY_BITS;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as above

Copy link
Contributor

@peterharperuk peterharperuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but see previous comments concerning minor changes

@peterharperuk peterharperuk added this to the 1.5.2 milestone Jan 12, 2024
@peterharperuk peterharperuk self-assigned this Jan 12, 2024
@kilograham kilograham modified the milestones: 1.6.0, 1.7.0 Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants