Hi Vipul, > On May 5, 2017, at 7:10 PM, Vipul Rahane <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >>>>>> wrote: >>>>>> >>>>>> On Thu, May 4, 2017 at 3:39 PM, Vipul Rahane <[email protected]> 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 >>>>> >>>> >>>> >> >
