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 <[email protected]> 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 <[email protected]> wrote: > >> Hello, >> >>> On May 6, 2017, at 9:29 PM, marko kiiskila <[email protected]> wrote: >>> >>> 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? >> >> 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 <[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 >>>>>>>> >>>>>>> >>>>>>> >>>>> >>>> >>> >> >> Regards, >> Vipul Rahane
