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();

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);


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.


I’ll comment on the pins and sys ids in a separate email thread.

Paul

On 3/28/16, 7:07 AM, "will sanfilippo" <wi...@runtime.io> wrote:

>I will second the motion to abbreviate things more :-) While I do like
>the simplicity of the mbed HAL I do realize that it does not support
>everything we want the HAL to do. And unless I am mistaken, it shouldnt
>be hard to map between the two. So I guess this means a +1 from me.
>
>Will
>
>> On Mar 27, 2016, at 2:29 PM, Sterling Hughes <sterl...@apache.org>
>>wrote:
>> 
>> Hey Paul,
>> 
>> I read through the APIs, I think they look good.  I made a few comments,
>> entirely coding standards related.
>> 
>> There are a few other things I'd like to understand/discuss, which I'll
>> post to dev@:
>> 
>> - Can you post a description of how pins are mapped across MCU, BSP and
>> Application?  I think I followed it, but want to make sure we have a
>> record.
>> 
>> - Should system device descriptor be preprocessor directives rather than
>> an enum.  Would there be a case where you'd want to do:
>> 
>> #ifdef SYSTEM_DEV_ADC5
>> /* do X */
>> #else
>> /* do Y */
>> #endif
>> 
>> - hal_adc_get_reference_voltage_mvolts i feel could be shortend to
>> hal_adc_refv() or hal_adc_refmv().  Shouldn't this just take a
>>resolution.
>> 
>> - hal_adc_val_convert_to_mvolts(), should this just be hal_adc_convert()
>> and take a resolution.
>> 
>> Cheers,
>> Sterling
>> 
>> On 3/25/16 4:52 PM, Paul Dietrich wrote:
>>> This new implementation is posted as
>>> 
>>> https://github.com/apache/incubator-mynewt-core/pull/25
>>> 
>>> Take a look and let me know what you think.  Without negative feedback,
>>> I’ll commit on Monday/Tuesday
>>> 
>>> Paul
>>> 
>>> On 3/25/16, 2:58 PM, "Paul Dietrich" <pa...@mistsys.com> wrote:
>>> 
>>>> Just updating the group with my plan
>>>> 
>>>> Folks commented offline that they didn¹t like that the mbed hal
>>>>doesn¹t
>>>> allow multiple kinds of devices with the same HAL API at the same
>>>>time.
>>>> 
>>>> But they liked the pin mapping and init function that tied it to a
>>>>pin.
>>>> 
>>>> So the new API will combine the best of hal_adc3 and hal_adc2.  I¹ll
>>>> hopefully post the pull request by the COB or during the weekend.
>>>> 
>>>> One NOTE.  The memory (RAM) issues of hal_adc2 will be addressed by
>>>> getting the device initializer from the BSP.  So the BSP may malloc
>>>>memory
>>>> for these to be efficient.
>>>> 
>>>> 
>>>> 
>>>> 
>>>>> 
>>>> 
>>>> 
>>> 
>>> 
>

Reply via email to