Hello, Answers inlined. These are my thoughts based on some discussion we had in house and earlier a few months ago.
> On Dec 18, 2017, at 2:58 PM, Jacob Rosenthal <[email protected]> wrote: > > Im trying to wrap my head around os devices and sensors and am finding the > current implementations odd. > > From what I can gather, os_dev_create creates an os device which mainly > provides the benefit of being able to open,suspend, resume,close a > resource. It does this by calling a callback (device init) function after > it finishes whom it expects to register the handler functions > > But oddly NOTHING in mynewt utilizes suspend or resume functionality, and > only adc, uart and pwm even register open/close handlers. Suspend and resume handlers should be used along with hal_bsp_power_state() which takes state as an input which would be either application specific or BSP specific. Looking at the state one could abstract when the os_dev_suspend() and os_dev_resume() get called fro ma state machine. These handlers would be implemented in the driver. Drivers can be very specific if needed or can be really generic. It all depends on what one needs in their product. Having said that, polling is the most basic configuration of a sensor that people start with along with various configuration options. It helps them in starting off and contribute to the OS in a meaningful manner. If some drivers do not contain deep sleep/suspend/resume functionality, contributors can help in adding such functionality to them. Note: I think we do not have OS_DEV_SETHANDLERS_PWR_STATE() or a similar macro to set handlers for suspend and resume which we would have to add. > Even more odd is that none of the slew of sensors registers ANY handlers > with os_device, and are therefore completely wasting the memory associated. Mynewt has a lot of optional stuff which might or might not be needed by all or part of hardware or BSP. > > Proof of this lies in the fact that only one out of the six upstream sensor > implementations (the shell implementations) even bothers to find and open > its existing device > https://github.com/apache/mynewt-core/blob/master/hw/drivers/sensors/bma253/src/bma253_shell.c > > <https://github.com/apache/mynewt-core/blob/master/hw/drivers/sensors/bma253/src/bma253_shell.c> This basically slipped through. It seems like a good use of os_dev, though. But, I would not want the sensor I am experimenting with to get suspended or resumed, I want raw access to the sensor to play with it. > > The other five leave the os device closed and they make a fresh interface. > https://github.com/apache/mynewt-core/blob/master/hw/drivers/sensors/bmp280/src/bmp280_shell.c#L39 > https://github.com/apache/mynewt-core/blob/master/hw/drivers/sensors/bno055/src/bno055_shell.c#L45 > https://github.com/apache/mynewt-core/blob/master/hw/drivers/sensors/bno055/src/bno055_shell.c#L45 > https://github.com/apache/mynewt-core/blob/master/hw/drivers/sensors/tcs34725/src/tcs34725_shell.c#L38 > https://github.com/apache/mynewt-core/blob/master/hw/drivers/sensors/tsl2561/src/tsl2561_shell.c#L55 > > <https://github.com/apache/mynewt-core/blob/master/hw/drivers/sensors/tsl2561/src/tsl2561_shell.c#L55> Reason for using a fresh interface is: The sensors come up with a fresh configuration. Shell is solely used for experimentation purposes/manufacturing. Configuration of a sensor happens in the BSP or in the sensor creator. This is the default configuration. When someone is experimenting with a sensor, they start off using the clean factory default config. Once the config is specified in the shell and all the sensors are experimented with, the developer would lock in certain configurations that they would want and do a re-configuration of the sensors at various points in the state machine. The name is what os_dev provides along with os_init callback that is used by other devices. It might/might not be useful dependent on what device you want to use it on. > > Which as I said, actually makes sense since opening doesnt do anything > > Which isnt to tear down any work by anyone involved, Its not like I offered > any input when this was all being merged originally :) > > Thoughts on what the usefulness of os_dev is and should be and thus how > sensors should or shouldn't be built on top of them? Also, we were talking about sensor_itf on Saturday. Do you think you have a plan in mind for that ? I do not see it being mentioned here. If you would love to contribute towards that, I have some implementation in mind: For the macro you were mentioning: ODI_GET_ITF() macro which takes the os device as an argument. SENSOR_GET_ITF() macro which takes the sensor as an argument would have to be changed to return os_dev->od_itf. For the os_dev_itf structure: os_dev_itf should be a separate structure and there should be a pointer to it in os_dev. For compatibility with public APIs using sensor_itf, in sensor.h we could do this: #define sensor_itf os_dev_itf Hope this clears up most of your doubts if not all. Others are welcome to chime in as well. os_devs existed before sensors did so, please feel free to correct me. Regards, Vipul Rahane
