GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — AOD fixup by mark9064 · Pull Request #2106 · InfiniTimeOrg/InfiniTime

Conversation

@mark9064

@mark9064 mark9064 commented Aug 23, 2024

Copy link
Copy Markdown
Member
  • Removed ROUNDED_DIV macro which doesn't make sense to rely on here (it's from the NRF SDK).
    • Would an namespace {} function be better than the inline one? Any ideas?
  • Motion is now updated when in AOD (as watchfaces display steps and this should update if in AOD).
  • Idle refresh rate increased from 2Hz to 8Hz. This reduces visible display strobing in AOD particularly on apps with a brighter background such as Twos

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
@github-actions

github-actions Bot commented Aug 23, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Build size and comparison to main:

Section Size Difference
text 374200B -32B
data 948B 0B
bss 63480B 0B
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ljahn ljahn mentioned this pull request Aug 23, 2024
fixes for Always On Display and SPI flash ID InfiniTimeOrg/InfiniSim#145
Merged
@mark9064
mark9064 marked this pull request as draft August 25, 2024 22:25
@mark9064

mark9064 commented Aug 25, 2024

Copy link
Copy Markdown
Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mark9064
mark9064 force-pushed the aod-fixup branch from 54148ff to ef09b34 Compare August 27, 2024 10:39
@mark9064 mark9064 mentioned this pull request Aug 31, 2024
When "Always On" is activated, after screen is inactive, measurements aren't updated #2117
Closed
1 task done
mark9064 added 4 commits September 22, 2024 16:06
@mark9064
8hz idle
aac9296
@mark9064
Refactor into defined states
839960d
@mark9064
Replace rounded div macro
3ebc25d
@mark9064
Improve sleep time calculation docs
7ced109
@mark9064
mark9064 force-pushed the aod-fixup branch from ef09b34 to 7ced109 Compare September 22, 2024 15:09
@mark9064
mark9064 marked this pull request as ready for review September 22, 2024 15:18
@mark9064

mark9064 commented Sep 22, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mark9064
Restrict hardware reactivation when not sleeping
e0870ec
NeroBurner
NeroBurner approved these changes Sep 28, 2024
View reviewed changes

@NeroBurner NeroBurner left a comment

Copy link
Copy Markdown
Contributor

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.

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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@NeroBurner NeroBurner added the enhancement Enhancement to an existing app/feature label Sep 28, 2024
@NeroBurner NeroBurner added this to the 1.15.0 milestone Sep 28, 2024
@JF002

JF002 commented Oct 27, 2024

Copy link
Copy Markdown
Collaborator

I've just measured the power usage with BLE disabled :

  • 1.14 for reference in sleep mode : 413µA
  • main in sleep mode : 400µA
  • main AOD : 3mA
  • aod-fixup (this PR) in sleep mode : 400µA
  • aod-fixup (this PR) in AOD mode : 3.2mA

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


@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

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JF002
JF002 approved these changes Oct 27, 2024
View reviewed changes

@JF002 JF002 left a comment

Copy link
Copy Markdown
Collaborator

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.

LGTM! We'll merge is as soon as @mark9064 confirm that the power usage is good enough!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mark9064

mark9064 commented Oct 27, 2024

Copy link
Copy Markdown
Member Author

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JF002

JF002 commented Oct 27, 2024

Copy link
Copy Markdown
Collaborator

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?

Yeah sure, just open a new PR when it's ready ;)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JF002
JF002 merged commit 8a2ee43 into InfiniTimeOrg:main Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JF002 JF002 JF002 approved these changes
@NeroBurner NeroBurner NeroBurner approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

enhancement Enhancement to an existing app/feature

Projects

None yet

Milestone

1.15.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@mark9064 @JF002 @NeroBurner
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.