Skip to content
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

[3.5 | Switch] Implement LibNX psm.h for power_switch.cpp #5

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

halotroop2288
Copy link
Member

It handles everything but power_seconds_left.

I haven't tested this, it could be broken.

@halotroop2288 halotroop2288 self-assigned this Jan 13, 2024
@halotroop2288 halotroop2288 added the enhancement New feature or request label Jan 13, 2024
@halotroop2288 halotroop2288 marked this pull request as ready for review January 21, 2024 01:03
@halotroop2288
Copy link
Member Author

I'd appreciate it if someone could test this on a real Switch, or at least help me test it myself.
I need a Godot project that can read out the power data... Then I guess I can just leave my Switch on for a few hours.

Copy link

@hokaze hokaze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me from what I can piece together from the incredibly sparse docs from libnx psm.h (which made the libnx software keyboard page look detailed and verbose by comparison) and Godot's OS docs

Requested change:

I think for POWERSTATE_CHARGED the check should be
if (percentage == 100 && enough_power)

The godot docs describe that power state as not only being at full-charged, but still actively plugged in: "Plugged in, battery fully charged."

If battery percentage is at 100% but it's still running off battery, it should be POWESTATE_ON_BATTERY, I believe?

I'll be happy to try and make a quick test project in Godot so we can confirm the power state stuff works and test it on hardware.

@hokaze
Copy link

hokaze commented Jan 21, 2024

Made a quick test project that should hopefully suffice, displays the relevant info from get_power_state (as their human readable names, not just the int constants), get_power_seconds_left and get_power_percent_left.

In addition to displaying in the GUI of the game itself, also periodically (defaults to 5s, but can be changed) logs that data to stdout, for if you want to leave the switch running for a while to test that it actually reflects battery/state change over time

BatteryTest.zip

I've got other things to attend to IRL for a bit, so haven't gotten round to testing it with the pr/switch-power branch specifically yet, but will probably do so either within a few hours or later on Sunday.

EDIT: I've been ill and stuck in bed all Sunday, will get back to this later

@halotroop2288 halotroop2288 requested review from a team and removed request for Stary2001 January 22, 2024 16:09
@hokaze
Copy link

hokaze commented Jan 22, 2024

Finally got round to testing this (still ill, but figured I could at least check this briefly) but the results are...confusing.

TL;DR - incredibly inconsistent, got the expected result for charge + state only once and leaving the battery test running for a few minutes (some of which were even with it backgrounded on home screen) resulted in my Switch crashing and needing a hard shutdown (~15s of holding power button)

Test 1

Copied via nxlink, with the debugger attached, undocked
Battery Percentage: 5 (expected: 85%) - FAIL
Power State: POWERSTATE_CHARGING (expected: POWERSTATE_ON_BATTERY) - FAIL

Notes:

  1. No idea why the percentage is completely wrong. Powerstate being wrong I first thought was me not mapping the enums to strings properly in the BatteryTest project, but no, it's reporting powerstate 3, which is charging, despite being on battery.
  2. Might have to adjust the BatteryTest project, as it checks the get_power_* methods every frame and with the WARN_PRINT() call this spams output and drowns out the projects' own print calls to check the other vars

Test 2

Ran again without the debugger attached, running straight off the SD card
Battery Percentage: 84 (expected: 84%) - PASS
Power State: POWERSTATE_ON_BATTERY (expected: POWERSTATE_ON_BATTERY) - PASS

Notes:

  1. All results as expected. Is...having the debugger attached somehow affecting results? Worth retesting both cases to see if the behaviour is consistent.

Test 3

local, no debugger, on battery
Battery Percentage: 119 (expected: 81%) - FAIL
Power State: POWERSTATE_CHARGING (expected: POWERSTATE_ON_BATTERY) - FAIL

...how??? How did we get 119? How did it work the last time with the exact number and correct state but not this time?

Test 4

local, no debugger, docked/charging
Battery Percentage: 37 (expected: 81%) - FAIL
Power State: POWERSTATE_CHARGING (expected: POWERSTATE_CHARGING) - PASS

Left the switch alone (in home menu) so it'd actually gain some charge...
...however, despite the charge increasing by 2%, the Godot application reported no change in percentage (despite it being polled and updating the labels every frame), even left it running another minute or so just to see if there was a delay, but nope, nothing.

Went back to the living room where my switch was docked to try and run another test and found it had actually crashed/froze on the BatteryTest screen - quit button, home button and power button were unresponsive, screen went black when undocked/redocked.

Had to hold down power for a hard shutdown, having to check the joycons to see if they were still connected or not to tell if it had actually turned off and if I could release the button.

As doing this has forced me to hunt for where I put my rcm jig in order to get back into atmosphere, and I'm still ill, probably won't be any more testing from me today.

I have no idea what's going on here - I suspect libnx's sparse documentation on how the power related functions may have led to some weirdness, but even if the battery percentage is not actually a regular int of 1-100 representing percentage charge like it should be, I've no idea how charges of 80-85% resulted in 5, 84, 119, 37.

I might try tweaking the battery test project to not update literally every frame, in case that's somehow related to the crash (even though it shouldn't be! I've ran some test projects and actual small games without issue), but right now I'm stumped as to why it froze like that.

(EDIT: I have attached an archive containing the .nro I used for testing, to save folks having to pull the changes from this PR, compile, update the templates, grab the test project + export)
BatteryTest.zip

@halotroop2288
Copy link
Member Author

halotroop2288 commented Jan 22, 2024

I can't thank you enough for doing these tests, @hokaze.
I really only requested review as a formality, and I don't believe I have even implemented your suggested changes yet, which I fully agree with. I'll be implementing them as soon as possible after I finish working on this new PR for additional software keyboard types.

resulted in my Switch crashing and needing a hard shutdown (~15s of holding power button)

This sounds like a memory leak. I really hope that's not our fault because I'm not trained well enough to debug a problem like that.

Battery Percentage: 119 (expected: 81%) - FAIL

That is extremely concerning.

@halotroop2288 halotroop2288 force-pushed the pr/switch-power branch 3 times, most recently from 6d49bf5 to 0c67054 Compare January 22, 2024 20:36
It handles everything but power_seconds_left.
It served little to no purpose.
@halotroop2288
Copy link
Member Author

halotroop2288 commented Jan 24, 2024

Last one. I deleted the extra files because the extra class wasn't serving much of a purpose.

I just want to be sure it still actually works since I moved the initializer and added a psmExit call.

I tested it myself. No issues!

@halotroop2288 halotroop2288 changed the title [Switch] Implement LibNX psm.h for power_switch.cpp [3.5 | Switch] Implement LibNX psm.h for power_switch.cpp Jan 24, 2024
@halotroop2288 halotroop2288 merged commit 8b85723 into Homebrodot:main/3.5 Jan 24, 2024
15 checks passed
@halotroop2288 halotroop2288 deleted the pr/switch-power branch January 24, 2024 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants