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 <jakerosent...@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 <jakerosent...@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 >>>> >>> >>> >
