pebble-dev / RebbleOS

open source operating system for low-power smartwatches
Other
355 stars 38 forks source link

RebbleOS Emu-Compass Support #163

Open tertty opened 2 weeks ago

tertty commented 2 weeks ago

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:

Data for the compass on the Pebble is also sent through the PQ channel with a unique PQP protocol ID. The pebble tool in the SDK includes a command for sending compass orientation (in degrees) and calibration status.

On the Pebble side, we have a custom handler for PQP compass packets that simply extracts the heading and calibration status from the packet and sends an event to the system with this information – which is exactly how the real compass driver hooks into the system.

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.

hardware "driver"/handler/whatever in rcore --- \
                                                 ---informs--> API service --informs--> system/user app
             QEMU protocol handler          --- /

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 Marcos Macros 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.

tertty commented 2 weeks ago

@jwise Thanks for the feedback and the compliment, both are appreciated.

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?

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 think architecturally, the concern is that it looks like the compass_service_subscribed 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 unsubscribes it.

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 power_update_battery() found in rcore/power.c. We can see in the runloop for the OS, this function gets called every cycle to get the latest information from the battery "driver". When the function updates that information, we notify the Service using battery_state_service_state_change() function. That function in turn updates the Service _state_handler with the latest battery information. I'm trying to achieve the same thing with my CompassService.

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 compass_service_state_change() which refreshes the Service's _state_handler. My goal from here would be to implement the hardware "driver" and then have that driver pop the information up to rcore/compass.c using the same corresponding functions as QEMU does, then notify the Service of some sort of change with the Compass via the compass_service_state_change().... and so on and so forth.

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).