On 3/28/16 9:44 AM, [email protected] wrote:
> Thanks for the reviews.
>
> 1) regarding names. I can shorten. I really like having the units in the
> names, but can shorted to
>
> hal_adc_refmv();
> hal_adc_bits();
OK.
I'm fine if we chose milivolts as our primary unit. But if we believe
MV will always be sufficient, why include it in the name? If we don't
think it will always be sufficient, we should have a generic call, i.e.
I think:
rc = hal_adc_ref(HAL_ADC_MV, &v);
is equally clear, and saves multiple functions for multiple unit types.
>
> As far as convert. To convert from adc output to molts requires the
> resolution and the reference voltage. I agree though that
> This should be a helper and not a driver Api. I’ll create a function
>
> hal_adc_util_to_mv(int val, int ref, int res);
>
>
Why not just hal_adc_convert (or convertmv), and don't make it a driver
function?
> As far as macros versus enums, I think either can do
>
> Enum val {
> #ifdef SYSTEM_DEV_ENABLED_ADCS
> SYSID_ADC_TEMP,
> SYSID_ADC_VOLTAGE,
>
> #endif
> ...
> };
>
> I personally like enums because there is warnings when case statements
> don’t address them and there are not as many compile time errors.
>
It looks to me like what you're proposing above is to have two sets of
defines. Whereas if you made those '#defines' you'd have one set.
Anyhow, I don't feel too strongly on this one. This definitely fits the
definition of an enum, so I can see the logic.
Sterling
