InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.65k stars 912 forks source link

motioncontroller: Clean up class and remove unused vars #1659

Closed FintasticMan closed 1 year ago

FintasticMan commented 1 year ago

This PR cleans up the MotionController class and some of the code surrounding it. None of the functionality is changed at all.

It also updates the names of some of the variables to match our style guide.

github-actions[bot] commented 1 year ago
Build size and comparison to main: Section Size Difference
text 406620B -80B
data 940B 0B
bss 53568B 0B
Riksu9000 commented 1 year ago

There are quite a few changes here. The ShakeWake changes and RaiseWake variable name change I'm not sure about.

FintasticMan commented 1 year ago

It's quite difficult to split these changes into separate commits because of how intertwined they are.

I think that lastYForRaiseWake is actually a better name for the variable, so I'll rename it to that. I would have liked to have just used the lastY variable, but that's not easy to do without changing any behaviour.

The ShakeWake changes make the code easier to read IMO. The only potential behaviour change is that if the watch is put to sleep at the same time the shake wake option is turned on, the time delta is lower than it would have been before this change. This means that it's possible for it to wake up immediately after going to sleep if the user is shaking the watch. Before it would only wake up a hundred milliseconds or so later. Seeing as this is a practically impossible situation (and isn't really an issue either way), and I can't see any other changes that impact behaviour, I think that this can be considered functionally the same

Riksu9000 commented 1 year ago

I don't see how they're that intertwined. It's difficult to isolate what's been moved where with unrelated changes, while also reasoning about them.

FintasticMan commented 1 year ago

@Riksu9000 I've just split my changes into separate commits, so now it should be easier to understand what's going on.