GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||
GeezerVison is showing github.com — AOD fixup by mark9064 · Pull Request #2106 · InfiniTimeOrg/InfiniTime Conversation
Member
Before merging this the power impact of the 8Hz idle commit needs to be tested; if it uses a lot more energy then it might not be worth the improved visuals. I don't have a power profiler so I'd really appreciate if someone could compare AOD power usage with that commit applied directly to main compared to current main (other commits in this branch like motion updates could affect results)
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
|
There was an error while loading. Please reload this page.
|
Build size and comparison to main:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This PR now depends on #2109 Looking at AOD again, re-using Idle in DisplayApp and Sleeping in SystemTask wasn't a great idea as these states now have 2 different meanings depending on whether AOD is enabled. Also, if something changes the AOD enablement state while in AOD, all hell breaks loose (currently this isn't possible, but an auto wake/sleep pr like is currently proposed would cause problems here) and generally a brittle design like this is something to avoid. So this PR will also add a new AODSleeping state to SystemTask and an AOD state to DisplayApp. But adding those states depends on the state refactor, as the waking up state gets in the way among other things |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Code changes are now ready to review The power of 8Hz idle should still be checked. Based off how battery life is for me it is worse but not by much, and noticeable flickering is resolved @JF002 you suggested a state diagram or similar in #2109 which I didn't have a chance to finish before merging. This PR adds a few states anyway so perhaps that was for the best! Anyway, what format do you think would work best? ASCII diagram in the code somewhere (if so where)? SVG state diagram? I'm not really sure how to present it |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I don't have a voltage meter. But the code changes are nice. More explicit AOD states is good in my opinion
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I've just measured the power usage with BLE disabled :
So there's a slight increase in power usage with this PR (3.0 -> 3.2mA). I guess this increase comes from the 8Hz refresh rate. I'll let @mark9064 decide if this is a acceptable compromise :)
Good question! I like PlantUML diagrams because they are "diagram as code" : easy to edit and easy to integrate in Git. But feel free to use/suggest any other diagram system ;-) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM! We'll merge is as soon as @mark9064 confirm that the power usage is good enough!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for the power testing 😄 Yep that lines up well with my experience in terms of battery life, it's a minor hit but well worth it to avoid strobing IMO I'm happy to do a PlantUML (I've used it before actually), I guess I'll queue up a new PR when that's done? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Yeah sure, just open a new PR when it's ready ;) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.