acassis commented on PR #17363:
URL: https://github.com/apache/nuttx/pull/17363#issuecomment-3567019767

   > This is a nicely formatted PR with a good description, testing and 
well-written documentation! Nice job!
   > 
   > I think the uORB addition seems fine, but I don't approve of merging this 
PR with the legacy sensor interface. I think this driver should be uORB-based 
only.
   > 
   > NuttX is moving to just the uORB interface for sensors. Having multiple 
interfaces is confusing to users, difficult to maintain and adds more 
complexity. The way it's implemented in this PR adds many functions as part of 
the public namespace where they are traditionally private (`static`) in other 
driver implementations. It gives users access to device struct contents and 
register definitions.
   > 
   > Is there a specific use case for the legacy sensor interface? If not, I 
suggest you move everything from `qmi8658_base.c` to `qmi8658_uorb.c` and 
remove all logic relating to the legacy sensor interface. The only public API 
for your sensor should be the `register` function, and possibly a few types if 
necessary for interacting with the sensor. Everything else should be done 
through existing uORB APIs/ioctl.
   
   @linguini1 I think char device sensor still important on NuttX because for 
boards with low resource adding uORB will be too much


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to