Hi Jacob, Lukasz,
Sorry for the late reply on this thread, just wanted to add on a few
thoughts. I definitely am open to improvements to OS device, it was
put in as the minimal framework for registering drivers in the system,
but it was by no-means complete when added.
- What I think OS device/driver interface has done well is that it
allows you to name a device, and have a public interface with multiple
implementations, depending on platform (e.g. hw/drivers/uart).
- Definitely missing from here is that locking of a device is currently
optional and left up to the driver implementor. I think we need a more
standard, and saner mechanism for locking a device on access and saying
whether a device is locked/waiting for device acquisition, etc.
- The suspend/resume functions on the drivers are called via
os_dev_suspend_all() and os_dev_resume_all() respectively. The original
thought is that these functions would be called within the context of
hal_bsp_power_state(), and it would be up to the implementor to call
those functions, if they wanted the drivers to go into lower power
states, and it would allow the drivers to save and restore state, while
the BSP was hard shutting down the system. I’ll admit that I don’t
think anybody has _ever_ used this, so it probably could use some love
:-) In my view, the solution here is to experiment with this, and have
some sample code so that we can figure out if the APIs are sufficient,
and if they are sufficient we can give people a reference point for
doing this themselves.
Not sure if this helps provide some clarity / definitely open to
thoughts.
Best,
Sterling
On 19 Dec 2017, at 6:17, Łukasz Rymanowski wrote:
Hi Jacob
I've look around this code and here is my two cents to the topic :)
On 18 December 2017 at 23:58, 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.
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.
Yup, we are missing suspend/resume support as for now. Meaning,
suspend/resume will not be never called and sadly we are missing even
API to set suspend/resume callbacks as Vipul mentioned.
However if you build new driver for something what can sleep, it is
probably good idea to add struct *os_dev to your driver struct and be
prepared for using it.
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
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
As Vipul mentioned those shells are used only for sensor testing. As
you probably noticed it will not use sensor API at all but instead it
uses direct access to the sensor_driver. If you want to use sensors
via sensor API you should look into sensor_shell from sensor_test
application
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?
Sensors now uses os_dev and this is good approach as we want them to
be aware of power state eventually. There is still lot to do, along
with full suspend/resume support and patches are always welcome.
hth
Best
Łukasz
