You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mynewt.apache.org by Jacob Rosenthal <ja...@gmail.com> on 2017/05/04 21:59:41 UTC
Re: nrf52dk hal_bsp_init() and sensor device creation
Hey Vipul et al,
Ive been plugging away at a few sensors (bma250, lis2dh) and so taking a
much closer look at this now.
It seems odd to me that sensors are getting pushed all way down to the BSP,
only to be created, but not inited. Also why dont BSP then pickup a
dependency on all those sensors? I presume Im missing something about the
os bringup?
Why not generalize to create maybe spi0 os_dev instead of accel os_dev then
since.. theres nothing accel specific being done in the bsp?
Thanks for any clarification.
On Fri, Mar 31, 2017 at 6:19 PM, Vipul Rahane <vi...@runtime.io> wrote:
> Yes, that’s what I mean’t. Thanks Will.
>
> -Vipul
>
> > On Mar 31, 2017, at 5:37 PM, will sanfilippo <wi...@runtime.io> wrote:
> >
> > I presume you mean “was getting too crowded” :-)
> >
> > Yeah, I copied that bsp to create this other bsp and noticed alot of
> sensor dev creates in there. Not a big deal but I think this will make the
> code a bit less crowded.
> >
> > +1
> >
> >
> >> On Mar 31, 2017, at 5:30 PM, Vipul Rahane <vi...@runtime.io> wrote:
> >>
> >> Hello,
> >>
> >> I am changing nrf52dk hal_bsp_init() so that it calls
> sensor_device_create() instead of creating sensors separately. this is
> because hal_bsp_init() was not getting too crowded.
> >>
> >> Regards,
> >> Vipul Rahane
> >
>
>
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by Vipul Rahane <vi...@runtime.io>.
Hello,
I have managed to get a working good enough solution for the two issues mentioned below. I have not opened a PR with this as I still have to make changes to all the drivers with the interface changes. The below comparison between master and my fork shows an example of how it would work:
https://github.com/apache/incubator-mynewt-core/compare/master...vrahane:sensor_imp?expand=1 <https://github.com/apache/incubator-mynewt-core/compare/master...vrahane:sensor_imp?expand=1>
This change in code in this fork basically do the following:
- Changa data structure to SLIST instead of TAILQ so that it can get
initialized statically.
- Remove dummy functions in the bsp and add the sensor device
initialization functions instead.
- resolve conflicts introduced by bme280 PR
- Add a seperate creator package for external sensors
- Add support for onboard sensor Vs external sensor by using syscfg
variables
- Add sensor interface struct so that it can get used by the individual
drivers so that we can talk to multiple sensors using the same driver
using same or different interfaces using hal.
This is how my targets/nrf52-sensors-test/syscfg.yml looks like:
> syscfg.vals:
> BME280_SHELL_CSPIN: "3"
> BME280_SHELL_ITF_NUM: "0"
> BME280_SHELL_ITF_TYPE: "0"
> LOG_LEVEL: "255"
> MSYS_1_BLOCK_COUNT: "30"
> NEWTMGR_BLE_HOST: "1"
> OC_LOGGING: "0"
> SENSOR_OIC_OBS_RATE: "2"
> SPI_0_MASTER: "1"
> BME280_ONB: "1"
I would soon get to a point where I would make these changes to all the drivers and add it to the MYNEWT-744 PR.
Suggestions/Comments are welcome.
Regards,
Vipul Rahane
> On May 16, 2017, at 11:25 AM, Vipul Rahane <vi...@runtime.io> wrote:
>
> Hello,
>
> Yes, I did speak to Marko about this. There were two things that came up as part of the discussion:
>
> 1. Creating a separate package for device creation.
> Since the sensors could be onboard or external, this might be something that is optional for sensors which are external. It could also be done in the app(I have to yet try this).
>
> 2. Propagate the interface number to the driver so that we can have multiple sensors using the same device driver without using the syscfg specific to the SPI driver. This I am experimenting with right now and it needs changes in the all the drivers. I have made changes to one driver and seems to work great. The only reason I am a bit worried about this is because it needs an extra argument for the interface in all the function calls for the driver which talk to the sensors using the hal. It might not be a big deal and maybe I am the only one worried about it :-)
>
> Also, the changes I have made so far as part of the PR do help partially by making the device creation possible in any part of the code.
>
> I will definitely post it on the dev list once I finalize on one approach which you can take a look at as a PR.
>
> Regards,
> Vipul Rahane
>
> On May 13, 2017, at 8:12 PM, Jacob Rosenthal <ja...@gmail.com> wrote:
>>
>> I saw theres a PR started last week
>> https://github.com/apache/incubator-mynewt-core/pull/258
>>
>> But looks like its still waiting on discussion?
>>
>> On Mon, May 8, 2017 at 12:12 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>
>>> Hello,
>>>
>>>> On May 6, 2017, at 9:29 PM, marko kiiskila <ma...@runtime.io> wrote:
>>>>
>>>> Hi Vipul,
>>>>
>>>>> On May 5, 2017, at 7:10 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>>>>
>>>>> I agree with Marko and I did try initializing the devices using the
>>> init callback in os_dev_create(). This kind of helps in removing the dummy
>>> functions.
>>>>> The fix required changing the data structure for the sensor_mgr sensor
>>> list to be a singly linked list. I am going to open a PR soon with the
>>> changes, after we have consensus on the point below:
>>>>>
>>>>> If we want to separate out device driver creation of board specific
>>> peripherals vs sensor chips, I have couple of suggestions:
>>>>>
>>>>> 1. hal_bsp_ext() which gets called after hal_bsp_init() probably based
>>> on priority.
>>>>
>>>> This I don’t understand. Could you expand how this would work?
>>>
>>> Here is what I was thinking:
>>>
>>> Have sysinit control the flow so that hal_bsp_init() gets called before
>>> hal_bsp_ext_init() but looking at it in detail it seems that would require
>>> a lot of changes including using sysinit for OS initialization which does
>>> not happen currently.
>>>
>>> Another way of doing what I was thinking about doing above is: We already
>>> have _KERNEL stage of devices which get initialized in os_pkt_init() which
>>> happens at the beginning of sysinit() which is always the first thing we do
>>> in main(). We can make hal_bsp_ext_pkg_init() which gets executed at stage
>>> 0 instead of os_pkt_init() and change the stage of os_pkg_init() to be 1.
>>>
>>> This approach would be similar to what I was thinking below. I guess this
>>> is the only way to do it without changing a lot of code in the OS.
>>>
>>>>
>>>>> 2. make sensor_dev_create() part of a separate independent pkg by
>>> itself which would have a priority that makes it get called after
>>> hal_bap_init(). This is because the sensor library might not be the right
>>> place to do the device creation for boards like the Ruuvi which do it in
>>> the bsp cause they have them onboard.
>>>>
>>>> I do understand the convenience of having just a single package where
>>>> you could declare os_dev’s for all plug-in sensors for which a driver
>>>> exists in -core. I think I’d prefer this option (I’m assuming 1 & 2 are
>>>> alternative approaches?)
>>>
>>> Yes, I was thinking of alternative approaches but that is not possible.
>>>
>>>>
>>>>>
>>>>> Regards,
>>>>> Vipul Rahane
>>>>>
>>>>>> On May 5, 2017, at 6:55 PM, marko kiiskila <ma...@runtime.io> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I did find it a bit odd to see these device initializations in nrf52dk
>>> BSP,
>>>>>> without matching dependency/syscfg variable declarations in there.
>>>>>>
>>>>>> I think what I’d want is to have BSP have the device creation only
>>>>>> if the board has the particular device on it. And if it has the device
>>>>>> creation, then it also should have the matching dependency to driver,
>>> as well
>>>>>> as the syscfg knob declaration to enable it.
>>>>>> Example of such a board is the Ruuvi Tag. It has a sensor on-board;
>>>>>> BSP should have a knob to enable it’s initialization. BSP also knows
>>>>>> how to reach that sensor. So it should be passing this data to driver
>>>>>> either with a syscfg setting, cfg structure in init or define in a
>>> header
>>>>>> file. One of those.
>>>>>>
>>>>>> nrf52dk does not have this hardware on it, so I feel that in that case
>>>>>> the OS device should be created somewhere else (in the app,
>>>>>> or separate package). Preferably this should happen relatively early,
>>>>>> so package initialized from sysinit() would already be able to
>>>>>> access that device. I don’t know what the right answer is here,
>>>>>> but I don’t think hardware added with jumper wires should be
>>>>>> initialized in the BSP itself.
>>>>>>
>>>>>> Also, it is not great to have empty init routines present for these
>>>>>> sensor devices. I did talk with Vipul about what kind of changes he’d
>>>>>> have to do to allow the driver _init routines to register the sensor
>>>>>> with the framework before the generic sensor framework is initialized.
>>>>>> I think he now has a handle on how to fix this.
>>>>>>
>>>>>> At least this is my current thinking. Anyone else with comments?
>>>>>>
>>>>>>> On May 5, 2017, at 3:18 PM, Jacob Rosenthal <ja...@gmail.com>
>>> wrote:
>>>>>>>
>>>>>>> Cool. So Ill let the bsp do the init.
>>>>>>>
>>>>>>> I am having one issue. For lis2dh, I need to know the current
>>> accel_mode to
>>>>>>> know if the data read back is 8, 10, or 12 bits. From
>>> lis2dh.c:sensor_read
>>>>>>> we get a sensor object and can lookup the config with
>>> SENSOR_GET_DEVICE and
>>>>>>> pass to lis2dh_get_vector_data. However when lis2dh_shell.c attemps to
>>>>>>> reuse that function it doesnt get passed a device, sensor, or a
>>> config ...
>>>>>>>
>>>>>>> I attempted (with my half ass c) to just grab the struct from the
>>> bsp, but
>>>>>>> its not currently in the header or global so Im getting a new
>>> instance and
>>>>>>> breaking my code. I could make the lis2dh device global in the bsp,
>>> but
>>>>>>> thats probably not preferred anyway. Is there an api Im missing, or
>>> one
>>>>>>> that fits here?
>>>>>>>
>>>>>>> relevant code in github
>>>>>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/
>>> c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/
>>> lis2dh/src/lis2dh.c#L372
>>>>>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/
>>> c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/
>>> lis2dh/src/lis2dh_shell.c#L98
>>>>>>>
>>>>>>>
>>>>>>> On Fri, May 5, 2017 at 11:42 AM, Kevin Townsend <ke...@adafruit.com>
>>> wrote:
>>>>>>>
>>>>>>>> This is my fault most likely ... the goal at the driver level was
>>> indeed
>>>>>>>> to init with a known default config, but obviously if people think a
>>>>>>>> different solution is better there's no reason not to change it!
>>>>>>>>
>>>>>>>> K.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 05/05/17 20:39, Vipul Rahane wrote:
>>>>>>>>
>>>>>>>>> I do not know the reason why it is done the way it is but I think
>>> you are
>>>>>>>>> right, the reason might have been to initialize with a default
>>> config.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On May 4, 2017, at 3:45 PM, Jacob Rosenthal <
>>> jakerosenthal@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io>
>>> wrote:
>>>>>>>>>>
>>>>>>>>>> I have been thinking about calling the driver initialization
>>> function
>>>>>>>>>>> directly from the bsp
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Can you comment on why you're not yet init in the BSP? Im thinking
>>> power
>>>>>>>>>> issues, which is another conversation I started in another thread.
>>> Maybe
>>>>>>>>>> if
>>>>>>>>>> the goal was to init with a config that was as low power as
>>> possible
>>>>>>>>>> maybe
>>>>>>>>>> its ok to do so in the BSP?
>>>>>>>>>>
>>>>>>>>>> May I know what do you mean by the following ? Isn’t it currently
>>> that
>>>>>>>>> way.
>>>>>>>>>
>>>>>>>>> Shouldnt these BSPs have dependencies on the drivers either way, if
>>> only
>>>>>>>>>> when the _PRESENT flag is on
>>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Vipul Rahane
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>> Regards,
>>> Vipul Rahane
>
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by Vipul Rahane <vi...@runtime.io>.
Hello,
Yes, I did speak to Marko about this. There were two things that came up as part of the discussion:
1. Creating a separate package for device creation.
Since the sensors could be onboard or external, this might be something that is optional for sensors which are external. It could also be done in the app(I have to yet try this).
2. Propagate the interface number to the driver so that we can have multiple sensors using the same device driver without using the syscfg specific to the SPI driver. This I am experimenting with right now and it needs changes in the all the drivers. I have made changes to one driver and seems to work great. The only reason I am a bit worried about this is because it needs an extra argument for the interface in all the function calls for the driver which talk to the sensors using the hal. It might not be a big deal and maybe I am the only one worried about it :-)
Also, the changes I have made so far as part of the PR do help partially by making the device creation possible in any part of the code.
I will definitely post it on the dev list once I finalize on one approach which you can take a look at as a PR.
Regards,
Vipul Rahane
On May 13, 2017, at 8:12 PM, Jacob Rosenthal <ja...@gmail.com> wrote:
>
> I saw theres a PR started last week
> https://github.com/apache/incubator-mynewt-core/pull/258
>
> But looks like its still waiting on discussion?
>
> On Mon, May 8, 2017 at 12:12 PM, Vipul Rahane <vi...@runtime.io> wrote:
>
>> Hello,
>>
>>> On May 6, 2017, at 9:29 PM, marko kiiskila <ma...@runtime.io> wrote:
>>>
>>> Hi Vipul,
>>>
>>>> On May 5, 2017, at 7:10 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>>>
>>>> I agree with Marko and I did try initializing the devices using the
>> init callback in os_dev_create(). This kind of helps in removing the dummy
>> functions.
>>>> The fix required changing the data structure for the sensor_mgr sensor
>> list to be a singly linked list. I am going to open a PR soon with the
>> changes, after we have consensus on the point below:
>>>>
>>>> If we want to separate out device driver creation of board specific
>> peripherals vs sensor chips, I have couple of suggestions:
>>>>
>>>> 1. hal_bsp_ext() which gets called after hal_bsp_init() probably based
>> on priority.
>>>
>>> This I don’t understand. Could you expand how this would work?
>>
>> Here is what I was thinking:
>>
>> Have sysinit control the flow so that hal_bsp_init() gets called before
>> hal_bsp_ext_init() but looking at it in detail it seems that would require
>> a lot of changes including using sysinit for OS initialization which does
>> not happen currently.
>>
>> Another way of doing what I was thinking about doing above is: We already
>> have _KERNEL stage of devices which get initialized in os_pkt_init() which
>> happens at the beginning of sysinit() which is always the first thing we do
>> in main(). We can make hal_bsp_ext_pkg_init() which gets executed at stage
>> 0 instead of os_pkt_init() and change the stage of os_pkg_init() to be 1.
>>
>> This approach would be similar to what I was thinking below. I guess this
>> is the only way to do it without changing a lot of code in the OS.
>>
>>>
>>>> 2. make sensor_dev_create() part of a separate independent pkg by
>> itself which would have a priority that makes it get called after
>> hal_bap_init(). This is because the sensor library might not be the right
>> place to do the device creation for boards like the Ruuvi which do it in
>> the bsp cause they have them onboard.
>>>
>>> I do understand the convenience of having just a single package where
>>> you could declare os_dev’s for all plug-in sensors for which a driver
>>> exists in -core. I think I’d prefer this option (I’m assuming 1 & 2 are
>>> alternative approaches?)
>>
>> Yes, I was thinking of alternative approaches but that is not possible.
>>
>>>
>>>>
>>>> Regards,
>>>> Vipul Rahane
>>>>
>>>>> On May 5, 2017, at 6:55 PM, marko kiiskila <ma...@runtime.io> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I did find it a bit odd to see these device initializations in nrf52dk
>> BSP,
>>>>> without matching dependency/syscfg variable declarations in there.
>>>>>
>>>>> I think what I’d want is to have BSP have the device creation only
>>>>> if the board has the particular device on it. And if it has the device
>>>>> creation, then it also should have the matching dependency to driver,
>> as well
>>>>> as the syscfg knob declaration to enable it.
>>>>> Example of such a board is the Ruuvi Tag. It has a sensor on-board;
>>>>> BSP should have a knob to enable it’s initialization. BSP also knows
>>>>> how to reach that sensor. So it should be passing this data to driver
>>>>> either with a syscfg setting, cfg structure in init or define in a
>> header
>>>>> file. One of those.
>>>>>
>>>>> nrf52dk does not have this hardware on it, so I feel that in that case
>>>>> the OS device should be created somewhere else (in the app,
>>>>> or separate package). Preferably this should happen relatively early,
>>>>> so package initialized from sysinit() would already be able to
>>>>> access that device. I don’t know what the right answer is here,
>>>>> but I don’t think hardware added with jumper wires should be
>>>>> initialized in the BSP itself.
>>>>>
>>>>> Also, it is not great to have empty init routines present for these
>>>>> sensor devices. I did talk with Vipul about what kind of changes he’d
>>>>> have to do to allow the driver _init routines to register the sensor
>>>>> with the framework before the generic sensor framework is initialized.
>>>>> I think he now has a handle on how to fix this.
>>>>>
>>>>> At least this is my current thinking. Anyone else with comments?
>>>>>
>>>>>> On May 5, 2017, at 3:18 PM, Jacob Rosenthal <ja...@gmail.com>
>> wrote:
>>>>>>
>>>>>> Cool. So Ill let the bsp do the init.
>>>>>>
>>>>>> I am having one issue. For lis2dh, I need to know the current
>> accel_mode to
>>>>>> know if the data read back is 8, 10, or 12 bits. From
>> lis2dh.c:sensor_read
>>>>>> we get a sensor object and can lookup the config with
>> SENSOR_GET_DEVICE and
>>>>>> pass to lis2dh_get_vector_data. However when lis2dh_shell.c attemps to
>>>>>> reuse that function it doesnt get passed a device, sensor, or a
>> config ...
>>>>>>
>>>>>> I attempted (with my half ass c) to just grab the struct from the
>> bsp, but
>>>>>> its not currently in the header or global so Im getting a new
>> instance and
>>>>>> breaking my code. I could make the lis2dh device global in the bsp,
>> but
>>>>>> thats probably not preferred anyway. Is there an api Im missing, or
>> one
>>>>>> that fits here?
>>>>>>
>>>>>> relevant code in github
>>>>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/
>> c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/
>> lis2dh/src/lis2dh.c#L372
>>>>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/
>> c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/
>> lis2dh/src/lis2dh_shell.c#L98
>>>>>>
>>>>>>
>>>>>> On Fri, May 5, 2017 at 11:42 AM, Kevin Townsend <ke...@adafruit.com>
>> wrote:
>>>>>>
>>>>>>> This is my fault most likely ... the goal at the driver level was
>> indeed
>>>>>>> to init with a known default config, but obviously if people think a
>>>>>>> different solution is better there's no reason not to change it!
>>>>>>>
>>>>>>> K.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 05/05/17 20:39, Vipul Rahane wrote:
>>>>>>>
>>>>>>>> I do not know the reason why it is done the way it is but I think
>> you are
>>>>>>>> right, the reason might have been to initialize with a default
>> config.
>>>>>>>>
>>>>>>>>
>>>>>>>> On May 4, 2017, at 3:45 PM, Jacob Rosenthal <
>> jakerosenthal@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io>
>> wrote:
>>>>>>>>>
>>>>>>>>> I have been thinking about calling the driver initialization
>> function
>>>>>>>>>> directly from the bsp
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Can you comment on why you're not yet init in the BSP? Im thinking
>> power
>>>>>>>>> issues, which is another conversation I started in another thread.
>> Maybe
>>>>>>>>> if
>>>>>>>>> the goal was to init with a config that was as low power as
>> possible
>>>>>>>>> maybe
>>>>>>>>> its ok to do so in the BSP?
>>>>>>>>>
>>>>>>>>> May I know what do you mean by the following ? Isn’t it currently
>> that
>>>>>>>> way.
>>>>>>>>
>>>>>>>> Shouldnt these BSPs have dependencies on the drivers either way, if
>> only
>>>>>>>>> when the _PRESENT flag is on
>>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Vipul Rahane
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>
>>>
>>
>> Regards,
>> Vipul Rahane
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by Jacob Rosenthal <ja...@gmail.com>.
I saw theres a PR started last week
https://github.com/apache/incubator-mynewt-core/pull/258
But looks like its still waiting on discussion?
On Mon, May 8, 2017 at 12:12 PM, Vipul Rahane <vi...@runtime.io> wrote:
> Hello,
>
> > On May 6, 2017, at 9:29 PM, marko kiiskila <ma...@runtime.io> wrote:
> >
> > Hi Vipul,
> >
> >> On May 5, 2017, at 7:10 PM, Vipul Rahane <vi...@runtime.io> wrote:
> >>
> >> I agree with Marko and I did try initializing the devices using the
> init callback in os_dev_create(). This kind of helps in removing the dummy
> functions.
> >> The fix required changing the data structure for the sensor_mgr sensor
> list to be a singly linked list. I am going to open a PR soon with the
> changes, after we have consensus on the point below:
> >>
> >> If we want to separate out device driver creation of board specific
> peripherals vs sensor chips, I have couple of suggestions:
> >>
> >> 1. hal_bsp_ext() which gets called after hal_bsp_init() probably based
> on priority.
> >
> > This I don’t understand. Could you expand how this would work?
>
> Here is what I was thinking:
>
> Have sysinit control the flow so that hal_bsp_init() gets called before
> hal_bsp_ext_init() but looking at it in detail it seems that would require
> a lot of changes including using sysinit for OS initialization which does
> not happen currently.
>
> Another way of doing what I was thinking about doing above is: We already
> have _KERNEL stage of devices which get initialized in os_pkt_init() which
> happens at the beginning of sysinit() which is always the first thing we do
> in main(). We can make hal_bsp_ext_pkg_init() which gets executed at stage
> 0 instead of os_pkt_init() and change the stage of os_pkg_init() to be 1.
>
> This approach would be similar to what I was thinking below. I guess this
> is the only way to do it without changing a lot of code in the OS.
>
> >
> >> 2. make sensor_dev_create() part of a separate independent pkg by
> itself which would have a priority that makes it get called after
> hal_bap_init(). This is because the sensor library might not be the right
> place to do the device creation for boards like the Ruuvi which do it in
> the bsp cause they have them onboard.
> >
> > I do understand the convenience of having just a single package where
> > you could declare os_dev’s for all plug-in sensors for which a driver
> > exists in -core. I think I’d prefer this option (I’m assuming 1 & 2 are
> > alternative approaches?)
>
> Yes, I was thinking of alternative approaches but that is not possible.
>
> >
> >>
> >> Regards,
> >> Vipul Rahane
> >>
> >>> On May 5, 2017, at 6:55 PM, marko kiiskila <ma...@runtime.io> wrote:
> >>>
> >>> Hi,
> >>>
> >>> I did find it a bit odd to see these device initializations in nrf52dk
> BSP,
> >>> without matching dependency/syscfg variable declarations in there.
> >>>
> >>> I think what I’d want is to have BSP have the device creation only
> >>> if the board has the particular device on it. And if it has the device
> >>> creation, then it also should have the matching dependency to driver,
> as well
> >>> as the syscfg knob declaration to enable it.
> >>> Example of such a board is the Ruuvi Tag. It has a sensor on-board;
> >>> BSP should have a knob to enable it’s initialization. BSP also knows
> >>> how to reach that sensor. So it should be passing this data to driver
> >>> either with a syscfg setting, cfg structure in init or define in a
> header
> >>> file. One of those.
> >>>
> >>> nrf52dk does not have this hardware on it, so I feel that in that case
> >>> the OS device should be created somewhere else (in the app,
> >>> or separate package). Preferably this should happen relatively early,
> >>> so package initialized from sysinit() would already be able to
> >>> access that device. I don’t know what the right answer is here,
> >>> but I don’t think hardware added with jumper wires should be
> >>> initialized in the BSP itself.
> >>>
> >>> Also, it is not great to have empty init routines present for these
> >>> sensor devices. I did talk with Vipul about what kind of changes he’d
> >>> have to do to allow the driver _init routines to register the sensor
> >>> with the framework before the generic sensor framework is initialized.
> >>> I think he now has a handle on how to fix this.
> >>>
> >>> At least this is my current thinking. Anyone else with comments?
> >>>
> >>>> On May 5, 2017, at 3:18 PM, Jacob Rosenthal <ja...@gmail.com>
> wrote:
> >>>>
> >>>> Cool. So Ill let the bsp do the init.
> >>>>
> >>>> I am having one issue. For lis2dh, I need to know the current
> accel_mode to
> >>>> know if the data read back is 8, 10, or 12 bits. From
> lis2dh.c:sensor_read
> >>>> we get a sensor object and can lookup the config with
> SENSOR_GET_DEVICE and
> >>>> pass to lis2dh_get_vector_data. However when lis2dh_shell.c attemps to
> >>>> reuse that function it doesnt get passed a device, sensor, or a
> config ...
> >>>>
> >>>> I attempted (with my half ass c) to just grab the struct from the
> bsp, but
> >>>> its not currently in the header or global so Im getting a new
> instance and
> >>>> breaking my code. I could make the lis2dh device global in the bsp,
> but
> >>>> thats probably not preferred anyway. Is there an api Im missing, or
> one
> >>>> that fits here?
> >>>>
> >>>> relevant code in github
> >>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/
> c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/
> lis2dh/src/lis2dh.c#L372
> >>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/
> c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/
> lis2dh/src/lis2dh_shell.c#L98
> >>>>
> >>>>
> >>>> On Fri, May 5, 2017 at 11:42 AM, Kevin Townsend <ke...@adafruit.com>
> wrote:
> >>>>
> >>>>> This is my fault most likely ... the goal at the driver level was
> indeed
> >>>>> to init with a known default config, but obviously if people think a
> >>>>> different solution is better there's no reason not to change it!
> >>>>>
> >>>>> K.
> >>>>>
> >>>>>
> >>>>>
> >>>>> On 05/05/17 20:39, Vipul Rahane wrote:
> >>>>>
> >>>>>> I do not know the reason why it is done the way it is but I think
> you are
> >>>>>> right, the reason might have been to initialize with a default
> config.
> >>>>>>
> >>>>>>
> >>>>>> On May 4, 2017, at 3:45 PM, Jacob Rosenthal <
> jakerosenthal@gmail.com>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io>
> wrote:
> >>>>>>>
> >>>>>>> I have been thinking about calling the driver initialization
> function
> >>>>>>>> directly from the bsp
> >>>>>>>>
> >>>>>>>
> >>>>>>> Can you comment on why you're not yet init in the BSP? Im thinking
> power
> >>>>>>> issues, which is another conversation I started in another thread.
> Maybe
> >>>>>>> if
> >>>>>>> the goal was to init with a config that was as low power as
> possible
> >>>>>>> maybe
> >>>>>>> its ok to do so in the BSP?
> >>>>>>>
> >>>>>>> May I know what do you mean by the following ? Isn’t it currently
> that
> >>>>>> way.
> >>>>>>
> >>>>>> Shouldnt these BSPs have dependencies on the drivers either way, if
> only
> >>>>>>> when the _PRESENT flag is on
> >>>>>>>
> >>>>>> Regards,
> >>>>>> Vipul Rahane
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>
> >
>
> Regards,
> Vipul Rahane
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by Vipul Rahane <vi...@runtime.io>.
Hello,
> On May 6, 2017, at 9:29 PM, marko kiiskila <ma...@runtime.io> wrote:
>
> Hi Vipul,
>
>> On May 5, 2017, at 7:10 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>
>> I agree with Marko and I did try initializing the devices using the init callback in os_dev_create(). This kind of helps in removing the dummy functions.
>> The fix required changing the data structure for the sensor_mgr sensor list to be a singly linked list. I am going to open a PR soon with the changes, after we have consensus on the point below:
>>
>> If we want to separate out device driver creation of board specific peripherals vs sensor chips, I have couple of suggestions:
>>
>> 1. hal_bsp_ext() which gets called after hal_bsp_init() probably based on priority.
>
> This I don’t understand. Could you expand how this would work?
Here is what I was thinking:
Have sysinit control the flow so that hal_bsp_init() gets called before hal_bsp_ext_init() but looking at it in detail it seems that would require a lot of changes including using sysinit for OS initialization which does not happen currently.
Another way of doing what I was thinking about doing above is: We already have _KERNEL stage of devices which get initialized in os_pkt_init() which happens at the beginning of sysinit() which is always the first thing we do in main(). We can make hal_bsp_ext_pkg_init() which gets executed at stage 0 instead of os_pkt_init() and change the stage of os_pkg_init() to be 1.
This approach would be similar to what I was thinking below. I guess this is the only way to do it without changing a lot of code in the OS.
>
>> 2. make sensor_dev_create() part of a separate independent pkg by itself which would have a priority that makes it get called after hal_bap_init(). This is because the sensor library might not be the right place to do the device creation for boards like the Ruuvi which do it in the bsp cause they have them onboard.
>
> I do understand the convenience of having just a single package where
> you could declare os_dev’s for all plug-in sensors for which a driver
> exists in -core. I think I’d prefer this option (I’m assuming 1 & 2 are
> alternative approaches?)
Yes, I was thinking of alternative approaches but that is not possible.
>
>>
>> Regards,
>> Vipul Rahane
>>
>>> On May 5, 2017, at 6:55 PM, marko kiiskila <ma...@runtime.io> wrote:
>>>
>>> Hi,
>>>
>>> I did find it a bit odd to see these device initializations in nrf52dk BSP,
>>> without matching dependency/syscfg variable declarations in there.
>>>
>>> I think what I’d want is to have BSP have the device creation only
>>> if the board has the particular device on it. And if it has the device
>>> creation, then it also should have the matching dependency to driver, as well
>>> as the syscfg knob declaration to enable it.
>>> Example of such a board is the Ruuvi Tag. It has a sensor on-board;
>>> BSP should have a knob to enable it’s initialization. BSP also knows
>>> how to reach that sensor. So it should be passing this data to driver
>>> either with a syscfg setting, cfg structure in init or define in a header
>>> file. One of those.
>>>
>>> nrf52dk does not have this hardware on it, so I feel that in that case
>>> the OS device should be created somewhere else (in the app,
>>> or separate package). Preferably this should happen relatively early,
>>> so package initialized from sysinit() would already be able to
>>> access that device. I don’t know what the right answer is here,
>>> but I don’t think hardware added with jumper wires should be
>>> initialized in the BSP itself.
>>>
>>> Also, it is not great to have empty init routines present for these
>>> sensor devices. I did talk with Vipul about what kind of changes he’d
>>> have to do to allow the driver _init routines to register the sensor
>>> with the framework before the generic sensor framework is initialized.
>>> I think he now has a handle on how to fix this.
>>>
>>> At least this is my current thinking. Anyone else with comments?
>>>
>>>> On May 5, 2017, at 3:18 PM, Jacob Rosenthal <ja...@gmail.com> wrote:
>>>>
>>>> Cool. So Ill let the bsp do the init.
>>>>
>>>> I am having one issue. For lis2dh, I need to know the current accel_mode to
>>>> know if the data read back is 8, 10, or 12 bits. From lis2dh.c:sensor_read
>>>> we get a sensor object and can lookup the config with SENSOR_GET_DEVICE and
>>>> pass to lis2dh_get_vector_data. However when lis2dh_shell.c attemps to
>>>> reuse that function it doesnt get passed a device, sensor, or a config ...
>>>>
>>>> I attempted (with my half ass c) to just grab the struct from the bsp, but
>>>> its not currently in the header or global so Im getting a new instance and
>>>> breaking my code. I could make the lis2dh device global in the bsp, but
>>>> thats probably not preferred anyway. Is there an api Im missing, or one
>>>> that fits here?
>>>>
>>>> relevant code in github
>>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/lis2dh/src/lis2dh.c#L372
>>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/lis2dh/src/lis2dh_shell.c#L98
>>>>
>>>>
>>>> On Fri, May 5, 2017 at 11:42 AM, Kevin Townsend <ke...@adafruit.com> wrote:
>>>>
>>>>> This is my fault most likely ... the goal at the driver level was indeed
>>>>> to init with a known default config, but obviously if people think a
>>>>> different solution is better there's no reason not to change it!
>>>>>
>>>>> K.
>>>>>
>>>>>
>>>>>
>>>>> On 05/05/17 20:39, Vipul Rahane wrote:
>>>>>
>>>>>> I do not know the reason why it is done the way it is but I think you are
>>>>>> right, the reason might have been to initialize with a default config.
>>>>>>
>>>>>>
>>>>>> On May 4, 2017, at 3:45 PM, Jacob Rosenthal <ja...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>>>>>>
>>>>>>> I have been thinking about calling the driver initialization function
>>>>>>>> directly from the bsp
>>>>>>>>
>>>>>>>
>>>>>>> Can you comment on why you're not yet init in the BSP? Im thinking power
>>>>>>> issues, which is another conversation I started in another thread. Maybe
>>>>>>> if
>>>>>>> the goal was to init with a config that was as low power as possible
>>>>>>> maybe
>>>>>>> its ok to do so in the BSP?
>>>>>>>
>>>>>>> May I know what do you mean by the following ? Isn’t it currently that
>>>>>> way.
>>>>>>
>>>>>> Shouldnt these BSPs have dependencies on the drivers either way, if only
>>>>>>> when the _PRESENT flag is on
>>>>>>>
>>>>>> Regards,
>>>>>> Vipul Rahane
>>>>>>
>>>>>
>>>>>
>>>
>>
>
Regards,
Vipul Rahane
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by marko kiiskila <ma...@runtime.io>.
Hi Vipul,
> On May 5, 2017, at 7:10 PM, Vipul Rahane <vi...@runtime.io> wrote:
>
> I agree with Marko and I did try initializing the devices using the init callback in os_dev_create(). This kind of helps in removing the dummy functions.
> The fix required changing the data structure for the sensor_mgr sensor list to be a singly linked list. I am going to open a PR soon with the changes, after we have consensus on the point below:
>
> If we want to separate out device driver creation of board specific peripherals vs sensor chips, I have couple of suggestions:
>
> 1. hal_bsp_ext() which gets called after hal_bsp_init() probably based on priority.
This I don’t understand. Could you expand how this would work?
> 2. make sensor_dev_create() part of a separate independent pkg by itself which would have a priority that makes it get called after hal_bap_init(). This is because the sensor library might not be the right place to do the device creation for boards like the Ruuvi which do it in the bsp cause they have them onboard.
I do understand the convenience of having just a single package where
you could declare os_dev’s for all plug-in sensors for which a driver
exists in -core. I think I’d prefer this option (I’m assuming 1 & 2 are
alternative approaches?)
>
> Regards,
> Vipul Rahane
>
>> On May 5, 2017, at 6:55 PM, marko kiiskila <ma...@runtime.io> wrote:
>>
>> Hi,
>>
>> I did find it a bit odd to see these device initializations in nrf52dk BSP,
>> without matching dependency/syscfg variable declarations in there.
>>
>> I think what I’d want is to have BSP have the device creation only
>> if the board has the particular device on it. And if it has the device
>> creation, then it also should have the matching dependency to driver, as well
>> as the syscfg knob declaration to enable it.
>> Example of such a board is the Ruuvi Tag. It has a sensor on-board;
>> BSP should have a knob to enable it’s initialization. BSP also knows
>> how to reach that sensor. So it should be passing this data to driver
>> either with a syscfg setting, cfg structure in init or define in a header
>> file. One of those.
>>
>> nrf52dk does not have this hardware on it, so I feel that in that case
>> the OS device should be created somewhere else (in the app,
>> or separate package). Preferably this should happen relatively early,
>> so package initialized from sysinit() would already be able to
>> access that device. I don’t know what the right answer is here,
>> but I don’t think hardware added with jumper wires should be
>> initialized in the BSP itself.
>>
>> Also, it is not great to have empty init routines present for these
>> sensor devices. I did talk with Vipul about what kind of changes he’d
>> have to do to allow the driver _init routines to register the sensor
>> with the framework before the generic sensor framework is initialized.
>> I think he now has a handle on how to fix this.
>>
>> At least this is my current thinking. Anyone else with comments?
>>
>>> On May 5, 2017, at 3:18 PM, Jacob Rosenthal <ja...@gmail.com> wrote:
>>>
>>> Cool. So Ill let the bsp do the init.
>>>
>>> I am having one issue. For lis2dh, I need to know the current accel_mode to
>>> know if the data read back is 8, 10, or 12 bits. From lis2dh.c:sensor_read
>>> we get a sensor object and can lookup the config with SENSOR_GET_DEVICE and
>>> pass to lis2dh_get_vector_data. However when lis2dh_shell.c attemps to
>>> reuse that function it doesnt get passed a device, sensor, or a config ...
>>>
>>> I attempted (with my half ass c) to just grab the struct from the bsp, but
>>> its not currently in the header or global so Im getting a new instance and
>>> breaking my code. I could make the lis2dh device global in the bsp, but
>>> thats probably not preferred anyway. Is there an api Im missing, or one
>>> that fits here?
>>>
>>> relevant code in github
>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/lis2dh/src/lis2dh.c#L372
>>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/lis2dh/src/lis2dh_shell.c#L98
>>>
>>>
>>> On Fri, May 5, 2017 at 11:42 AM, Kevin Townsend <ke...@adafruit.com> wrote:
>>>
>>>> This is my fault most likely ... the goal at the driver level was indeed
>>>> to init with a known default config, but obviously if people think a
>>>> different solution is better there's no reason not to change it!
>>>>
>>>> K.
>>>>
>>>>
>>>>
>>>> On 05/05/17 20:39, Vipul Rahane wrote:
>>>>
>>>>> I do not know the reason why it is done the way it is but I think you are
>>>>> right, the reason might have been to initialize with a default config.
>>>>>
>>>>>
>>>>> On May 4, 2017, at 3:45 PM, Jacob Rosenthal <ja...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>>>>>
>>>>>> I have been thinking about calling the driver initialization function
>>>>>>> directly from the bsp
>>>>>>>
>>>>>>
>>>>>> Can you comment on why you're not yet init in the BSP? Im thinking power
>>>>>> issues, which is another conversation I started in another thread. Maybe
>>>>>> if
>>>>>> the goal was to init with a config that was as low power as possible
>>>>>> maybe
>>>>>> its ok to do so in the BSP?
>>>>>>
>>>>>> May I know what do you mean by the following ? Isn’t it currently that
>>>>> way.
>>>>>
>>>>> Shouldnt these BSPs have dependencies on the drivers either way, if only
>>>>>> when the _PRESENT flag is on
>>>>>>
>>>>> Regards,
>>>>> Vipul Rahane
>>>>>
>>>>
>>>>
>>
>
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by Vipul Rahane <vi...@runtime.io>.
I agree with Marko and I did try initializing the devices using the init callback in os_dev_create(). This kind of helps in removing the dummy functions.
The fix required changing the data structure for the sensor_mgr sensor list to be a singly linked list. I am going to open a PR soon with the changes, after we have consensus on the point below:
If we want to separate out device driver creation of board specific peripherals vs sensor chips, I have couple of suggestions:
1. hal_bsp_ext() which gets called after hal_bsp_init() probably based on priority.
2. make sensor_dev_create() part of a separate independent pkg by itself which would have a priority that makes it get called after hal_bap_init(). This is because the sensor library might not be the right place to do the device creation for boards like the Ruuvi which do it in the bsp cause they have them onboard.
Regards,
Vipul Rahane
> On May 5, 2017, at 6:55 PM, marko kiiskila <ma...@runtime.io> wrote:
>
> Hi,
>
> I did find it a bit odd to see these device initializations in nrf52dk BSP,
> without matching dependency/syscfg variable declarations in there.
>
> I think what I’d want is to have BSP have the device creation only
> if the board has the particular device on it. And if it has the device
> creation, then it also should have the matching dependency to driver, as well
> as the syscfg knob declaration to enable it.
> Example of such a board is the Ruuvi Tag. It has a sensor on-board;
> BSP should have a knob to enable it’s initialization. BSP also knows
> how to reach that sensor. So it should be passing this data to driver
> either with a syscfg setting, cfg structure in init or define in a header
> file. One of those.
>
> nrf52dk does not have this hardware on it, so I feel that in that case
> the OS device should be created somewhere else (in the app,
> or separate package). Preferably this should happen relatively early,
> so package initialized from sysinit() would already be able to
> access that device. I don’t know what the right answer is here,
> but I don’t think hardware added with jumper wires should be
> initialized in the BSP itself.
>
> Also, it is not great to have empty init routines present for these
> sensor devices. I did talk with Vipul about what kind of changes he’d
> have to do to allow the driver _init routines to register the sensor
> with the framework before the generic sensor framework is initialized.
> I think he now has a handle on how to fix this.
>
> At least this is my current thinking. Anyone else with comments?
>
>> On May 5, 2017, at 3:18 PM, Jacob Rosenthal <ja...@gmail.com> wrote:
>>
>> Cool. So Ill let the bsp do the init.
>>
>> I am having one issue. For lis2dh, I need to know the current accel_mode to
>> know if the data read back is 8, 10, or 12 bits. From lis2dh.c:sensor_read
>> we get a sensor object and can lookup the config with SENSOR_GET_DEVICE and
>> pass to lis2dh_get_vector_data. However when lis2dh_shell.c attemps to
>> reuse that function it doesnt get passed a device, sensor, or a config ...
>>
>> I attempted (with my half ass c) to just grab the struct from the bsp, but
>> its not currently in the header or global so Im getting a new instance and
>> breaking my code. I could make the lis2dh device global in the bsp, but
>> thats probably not preferred anyway. Is there an api Im missing, or one
>> that fits here?
>>
>> relevant code in github
>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/lis2dh/src/lis2dh.c#L372
>> https://github.com/jacobrosenthal/mynewt-unofficial/blob/c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/lis2dh/src/lis2dh_shell.c#L98
>>
>>
>> On Fri, May 5, 2017 at 11:42 AM, Kevin Townsend <ke...@adafruit.com> wrote:
>>
>>> This is my fault most likely ... the goal at the driver level was indeed
>>> to init with a known default config, but obviously if people think a
>>> different solution is better there's no reason not to change it!
>>>
>>> K.
>>>
>>>
>>>
>>> On 05/05/17 20:39, Vipul Rahane wrote:
>>>
>>>> I do not know the reason why it is done the way it is but I think you are
>>>> right, the reason might have been to initialize with a default config.
>>>>
>>>>
>>>> On May 4, 2017, at 3:45 PM, Jacob Rosenthal <ja...@gmail.com>
>>>>> wrote:
>>>>>
>>>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>>>>
>>>>> I have been thinking about calling the driver initialization function
>>>>>> directly from the bsp
>>>>>>
>>>>>
>>>>> Can you comment on why you're not yet init in the BSP? Im thinking power
>>>>> issues, which is another conversation I started in another thread. Maybe
>>>>> if
>>>>> the goal was to init with a config that was as low power as possible
>>>>> maybe
>>>>> its ok to do so in the BSP?
>>>>>
>>>>> May I know what do you mean by the following ? Isn’t it currently that
>>>> way.
>>>>
>>>> Shouldnt these BSPs have dependencies on the drivers either way, if only
>>>>> when the _PRESENT flag is on
>>>>>
>>>> Regards,
>>>> Vipul Rahane
>>>>
>>>
>>>
>
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by marko kiiskila <ma...@runtime.io>.
Hi,
I did find it a bit odd to see these device initializations in nrf52dk BSP,
without matching dependency/syscfg variable declarations in there.
I think what I’d want is to have BSP have the device creation only
if the board has the particular device on it. And if it has the device
creation, then it also should have the matching dependency to driver, as well
as the syscfg knob declaration to enable it.
Example of such a board is the Ruuvi Tag. It has a sensor on-board;
BSP should have a knob to enable it’s initialization. BSP also knows
how to reach that sensor. So it should be passing this data to driver
either with a syscfg setting, cfg structure in init or define in a header
file. One of those.
nrf52dk does not have this hardware on it, so I feel that in that case
the OS device should be created somewhere else (in the app,
or separate package). Preferably this should happen relatively early,
so package initialized from sysinit() would already be able to
access that device. I don’t know what the right answer is here,
but I don’t think hardware added with jumper wires should be
initialized in the BSP itself.
Also, it is not great to have empty init routines present for these
sensor devices. I did talk with Vipul about what kind of changes he’d
have to do to allow the driver _init routines to register the sensor
with the framework before the generic sensor framework is initialized.
I think he now has a handle on how to fix this.
At least this is my current thinking. Anyone else with comments?
> On May 5, 2017, at 3:18 PM, Jacob Rosenthal <ja...@gmail.com> wrote:
>
> Cool. So Ill let the bsp do the init.
>
> I am having one issue. For lis2dh, I need to know the current accel_mode to
> know if the data read back is 8, 10, or 12 bits. From lis2dh.c:sensor_read
> we get a sensor object and can lookup the config with SENSOR_GET_DEVICE and
> pass to lis2dh_get_vector_data. However when lis2dh_shell.c attemps to
> reuse that function it doesnt get passed a device, sensor, or a config ...
>
> I attempted (with my half ass c) to just grab the struct from the bsp, but
> its not currently in the header or global so Im getting a new instance and
> breaking my code. I could make the lis2dh device global in the bsp, but
> thats probably not preferred anyway. Is there an api Im missing, or one
> that fits here?
>
> relevant code in github
> https://github.com/jacobrosenthal/mynewt-unofficial/blob/c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/lis2dh/src/lis2dh.c#L372
> https://github.com/jacobrosenthal/mynewt-unofficial/blob/c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/lis2dh/src/lis2dh_shell.c#L98
>
>
> On Fri, May 5, 2017 at 11:42 AM, Kevin Townsend <ke...@adafruit.com> wrote:
>
>> This is my fault most likely ... the goal at the driver level was indeed
>> to init with a known default config, but obviously if people think a
>> different solution is better there's no reason not to change it!
>>
>> K.
>>
>>
>>
>> On 05/05/17 20:39, Vipul Rahane wrote:
>>
>>> I do not know the reason why it is done the way it is but I think you are
>>> right, the reason might have been to initialize with a default config.
>>>
>>>
>>> On May 4, 2017, at 3:45 PM, Jacob Rosenthal <ja...@gmail.com>
>>>> wrote:
>>>>
>>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>>>
>>>> I have been thinking about calling the driver initialization function
>>>>> directly from the bsp
>>>>>
>>>>
>>>> Can you comment on why you're not yet init in the BSP? Im thinking power
>>>> issues, which is another conversation I started in another thread. Maybe
>>>> if
>>>> the goal was to init with a config that was as low power as possible
>>>> maybe
>>>> its ok to do so in the BSP?
>>>>
>>>> May I know what do you mean by the following ? Isn’t it currently that
>>> way.
>>>
>>> Shouldnt these BSPs have dependencies on the drivers either way, if only
>>>> when the _PRESENT flag is on
>>>>
>>> Regards,
>>> Vipul Rahane
>>>
>>
>>
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by Jacob Rosenthal <ja...@gmail.com>.
Cool. So Ill let the bsp do the init.
I am having one issue. For lis2dh, I need to know the current accel_mode to
know if the data read back is 8, 10, or 12 bits. From lis2dh.c:sensor_read
we get a sensor object and can lookup the config with SENSOR_GET_DEVICE and
pass to lis2dh_get_vector_data. However when lis2dh_shell.c attemps to
reuse that function it doesnt get passed a device, sensor, or a config ...
I attempted (with my half ass c) to just grab the struct from the bsp, but
its not currently in the header or global so Im getting a new instance and
breaking my code. I could make the lis2dh device global in the bsp, but
thats probably not preferred anyway. Is there an api Im missing, or one
that fits here?
relevant code in github
https://github.com/jacobrosenthal/mynewt-unofficial/blob/c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/lis2dh/src/lis2dh.c#L372
https://github.com/jacobrosenthal/mynewt-unofficial/blob/c83248e4eb063abbd4486401c6d2023b681ba5c6/hw/drivers/sensors/lis2dh/src/lis2dh_shell.c#L98
On Fri, May 5, 2017 at 11:42 AM, Kevin Townsend <ke...@adafruit.com> wrote:
> This is my fault most likely ... the goal at the driver level was indeed
> to init with a known default config, but obviously if people think a
> different solution is better there's no reason not to change it!
>
> K.
>
>
>
> On 05/05/17 20:39, Vipul Rahane wrote:
>
>> I do not know the reason why it is done the way it is but I think you are
>> right, the reason might have been to initialize with a default config.
>>
>>
>> On May 4, 2017, at 3:45 PM, Jacob Rosenthal <ja...@gmail.com>
>>> wrote:
>>>
>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>>
>>> I have been thinking about calling the driver initialization function
>>>> directly from the bsp
>>>>
>>>
>>> Can you comment on why you're not yet init in the BSP? Im thinking power
>>> issues, which is another conversation I started in another thread. Maybe
>>> if
>>> the goal was to init with a config that was as low power as possible
>>> maybe
>>> its ok to do so in the BSP?
>>>
>>> May I know what do you mean by the following ? Isn’t it currently that
>> way.
>>
>> Shouldnt these BSPs have dependencies on the drivers either way, if only
>>> when the _PRESENT flag is on
>>>
>> Regards,
>> Vipul Rahane
>>
>
>
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by Kevin Townsend <ke...@adafruit.com>.
This is my fault most likely ... the goal at the driver level was indeed
to init with a known default config, but obviously if people think a
different solution is better there's no reason not to change it!
K.
On 05/05/17 20:39, Vipul Rahane wrote:
> I do not know the reason why it is done the way it is but I think you are right, the reason might have been to initialize with a default config.
>
>
>> On May 4, 2017, at 3:45 PM, Jacob Rosenthal <ja...@gmail.com> wrote:
>>
>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>
>>> I have been thinking about calling the driver initialization function
>>> directly from the bsp
>>
>> Can you comment on why you're not yet init in the BSP? Im thinking power
>> issues, which is another conversation I started in another thread. Maybe if
>> the goal was to init with a config that was as low power as possible maybe
>> its ok to do so in the BSP?
>>
> May I know what do you mean by the following ? Isn’t it currently that way.
>
>> Shouldnt these BSPs have dependencies on the drivers either way, if only
>> when the _PRESENT flag is on
> Regards,
> Vipul Rahane
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by Vipul Rahane <vi...@runtime.io>.
I do not know the reason why it is done the way it is but I think you are right, the reason might have been to initialize with a default config.
> On May 4, 2017, at 3:45 PM, Jacob Rosenthal <ja...@gmail.com> wrote:
>
> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io> wrote:
>
>> I have been thinking about calling the driver initialization function
>> directly from the bsp
>
>
> Can you comment on why you're not yet init in the BSP? Im thinking power
> issues, which is another conversation I started in another thread. Maybe if
> the goal was to init with a config that was as low power as possible maybe
> its ok to do so in the BSP?
>
May I know what do you mean by the following ? Isn’t it currently that way.
> Shouldnt these BSPs have dependencies on the drivers either way, if only
> when the _PRESENT flag is on
Regards,
Vipul Rahane
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by Jacob Rosenthal <ja...@gmail.com>.
On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <vi...@runtime.io> wrote:
> I have been thinking about calling the driver initialization function
> directly from the bsp
Can you comment on why you're not yet init in the BSP? Im thinking power
issues, which is another conversation I started in another thread. Maybe if
the goal was to init with a config that was as low power as possible maybe
its ok to do so in the BSP?
Shouldnt these BSPs have dependencies on the drivers either way, if only
when the _PRESENT flag is on
Re: nrf52dk hal_bsp_init() and sensor device creation
Posted by Vipul Rahane <vi...@runtime.io>.
Hello Jacob,
There are two things you are mentioning:
1. For the initialization of the sensor chip: I have been thinking about calling the driver initialization function directly from the bsp. Right now there is a function which doesn’t to do anything and is just a placeholder cause there needs to be an initialization function for each driver. It is quite counter-intuitive I agree. Although, I think this has to do something w.r.t. sysinit and the priority level at which the devices and peripherals actually come up.
Also, why device creation is so deep is because we want the BSP to be the one source file where all the devices get created. They can be opened anywhere in the app to do further config.
2. "Why not generalize to create maybe spi0 os_dev instead of accel os_dev then since.. theres nothing accel specific being done in the bsp?”
the SensorAPI is a generic mechanism we are using to talk to sensors over different interfaces, these could be UART/I2C/SPI, etc. SPI0 is the actual interface which has a driver associated with it, every device on SPI0 cannot be named spi0. Hence, the unique identification string. Having said that, you could name your device anything which can ride SPI0.
With I2C, there could be multiple sensors that are connected to the same SDA, SCL.
Hope I answered your questions.
Regards,
Vipul Rahane
> On May 4, 2017, at 2:59 PM, Jacob Rosenthal <ja...@gmail.com> wrote:
>
> Hey Vipul et al,
>
> Ive been plugging away at a few sensors (bma250, lis2dh) and so taking a
> much closer look at this now.
>
> It seems odd to me that sensors are getting pushed all way down to the BSP,
> only to be created, but not inited. Also why dont BSP then pickup a
> dependency on all those sensors? I presume Im missing something about the
> os bringup?
>
> Why not generalize to create maybe spi0 os_dev instead of accel os_dev then
> since.. theres nothing accel specific being done in the bsp?
>
> Thanks for any clarification.
>
>
> On Fri, Mar 31, 2017 at 6:19 PM, Vipul Rahane <vi...@runtime.io> wrote:
>
>> Yes, that’s what I mean’t. Thanks Will.
>>
>> -Vipul
>>
>>> On Mar 31, 2017, at 5:37 PM, will sanfilippo <wi...@runtime.io> wrote:
>>>
>>> I presume you mean “was getting too crowded” :-)
>>>
>>> Yeah, I copied that bsp to create this other bsp and noticed alot of
>> sensor dev creates in there. Not a big deal but I think this will make the
>> code a bit less crowded.
>>>
>>> +1
>>>
>>>
>>>> On Mar 31, 2017, at 5:30 PM, Vipul Rahane <vi...@runtime.io> wrote:
>>>>
>>>> Hello,
>>>>
>>>> I am changing nrf52dk hal_bsp_init() so that it calls
>> sensor_device_create() instead of creating sensors separately. this is
>> because hal_bsp_init() was not getting too crowded.
>>>>
>>>> Regards,
>>>> Vipul Rahane
>>>
>>
>>