-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RebbleOS Emu-Compass Support #163
base: master
Are you sure you want to change the base?
Conversation
…ls, compass service, and qemu endpoints
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code quality looks great. I think architecturally, the concern is that it looks like the compass_service_subscribe
d routine gets called all the way from the QEMU packet handler (i.e., application client code is getting intermixed with the QEMU packet), and when the application quits, nothing unsubscribe
s it. I think that the handler for the compass_service
should push a message into the application's main loop to trigger a callback there? I haven't looked at any of this in (literally) years; I should hope we do this elsewhere, but do we?
@jwise Thanks for the feedback and the compliment, both are appreciated.
Upon reading your comment and looking at my code - as well as the project - again, I wonder if I've modeled this Service from the wrong example. Again, I chose to look at Berry's battery_state_service.c/.h files and model my design from there as I figured we had a similar mission of getting information from the hardware and popping it up to a Pebble Service via API. However, now that I've gone through this exercise and made similar realizations and pitfalls as Berry and probably you, I can now understand better your design of the tick_timer_service and how you've integrated that into the app manager. Is this the better path to follow, and if so could you help me understand the design of the app manager better and how services are integrated? If so, this would probably alleviate your concern and promote a more robust design.
I want to clarify this statement a little bit, because I think the details may be a little incorrect, but the overall concern is shared by me. By focusing on getting QEMU to work before hardware, I'm afraid of any bias that may have been created in my design. By following the design of the battery_state_service from the bottom up, I see that we have a mediator type service in rcore that is a level before the integration in rwatch, and a level after the os thread (in which this thread calls the hardware "driver" by proxy). For example, let's follow The QEMU handler calls the corresponding functions in my rcore/compass.c which then notifies the Service of some sort of change with the Compass via Does this design seem sane? Or are the layers not abstracted out enough from one another. I feel like my design for the homogeneous "bubble up" path of the data makes sense. And perhaps it's just the Service insertion into an app that needs work? Again, I feel like this models what we currently do with our battery with the exception of: we always need to call and get our battery level, but we only need the Compass when it's being subscribed to by an app (I don't think there's anything in the Pebble OS that has the Compass continuously running). |
The Plan
Get the CompassService to work on RebbleOS by creating the drivers/services required for the accelerometer and magnetometer. Additionally, implement the emu-compass commands found with the pebble CLT. This PR covers the latter (gotta walk before you can run).
Design
First and foremost, I have entirely based the design of this service from the currently implemented BatteryService. However, I don't know if this is the best idea or that I entirely understand why things are the way they are with the current BatteryService. If there is a rhyme or reason that could be explained to me, that would help.
While researching the CompassService and emu-compass features on the archived Pebble Development Website, I ran across this blog post discussing the design of the Pebble QEMU emulator. Towards the bottom of the post, I found this helpful tidbit that I used as a guiding light throughout design and implementation:
I found this helpful because this means there isn't a separate service or code that's dedicated to handling emu-compass data vs "real" compass data. All data should funnel down to the same core API functions. I sort of saw how we were doing this with our current BatteryService, and I hope I've created a good foundation that will allow data to be pushed up to the API and not have the API pull data from the system.
Implementation
A new compass.c/.h file in rcore has been created representing the routines for controlling the compass by combining the accelerometer and the magnetometer. A lot of this is currently just boilerplate for the (hopefully) later hardware implementation.
I have added a unique QEMU endpoint called QemuProtocol_Compass as I felt like the emu-compass was its own category and didn't fit in the current existing defined endpoints. Also, I felt the above quote was hinting towards this following the original implementation.
From there, a new protocol_compass.c/.h file has been added to our protocol folder in rcore. Inspired by the BatteryService, I've named this module "pcolcompass" and filed it under a "SYS" module type. This should properly bring out the two functions we need for emu-compass coms.
The following functions from the CompassService have graduated from UNIMPL to implemented:
I've also implemented the following Pebble
MarcosMacros that are heavily tied with the CompassService:Testing & Validation
I've attempted to run my verification tests using three different compass applications. I've chosen these apps specifically because A.) they looked good enough and B.) they had readily available source code that could be used to diagnose potential issues. Despite this, I unfortunately ran into a pitfall in nearly each one. I think not having the accelerometer and magnetometer and their Services (as well as other general API) rung out caused a lot of these snags:
In the following video demonstration you can find me testing the dummy compass emulation commands by comparing RebbleOS with Pebble. The top QEMU window is RebbleOS and the bottom QEMU window is Pebble. An obvious difference between the two windows is the missing compass needle in RebbleOS. I'm not exactly sure why this is. It could be a side effect from something else not yet being implemented, not sure. However I'm boldly asking you to ignore it, and instead pay attention to the degree reading on the top right of the application. As you can see, when I send the same command to RebbleOS and Pebble, the reading stays consistent.
Screencast from 2024-02-18 16-01-15.webm
Conclusion
Therefore according to the results of my test, I feel like I have enough to prove that the basic functionality of the emu-compass tool is working even with the snags. If possible, I'd like feedback on if I've inserted my CompassService cleanly with RebbleOS' existing Service design and if I've created a stable and foundation for later hardware implementation. Of course, all other general feedback is welcome too.