Skip to content
This repository has been archived by the owner on Feb 3, 2021. It is now read-only.

Have ACPI use only the first battery value #126

Merged
merged 8 commits into from
Jan 9, 2019
Merged

Have ACPI use only the first battery value #126

merged 8 commits into from
Jan 9, 2019

Conversation

matchai
Copy link
Owner

@matchai matchai commented Dec 28, 2018

Description

Ported the solution used in the following PR: spaceship-prompt/spaceship-prompt#583

Motivation and Context

Closes #124

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Screenshots (if appropriate):

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux

This feature is untested as I have no computer using acpi. I would appreciate it if someone could validate this fix.

Checklist:

  • I have checked that no other PR duplicates mine
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@matchai matchai requested a review from Snuggle December 28, 2018 00:20
Snuggle
Snuggle approved these changes Dec 28, 2018
View reviewed changes
Copy link
Collaborator

@Snuggle Snuggle 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!
Apologies for being away during the holidays and I hope you enjoyed yours! ☃️💖

@Snuggle
Copy link
Collaborator

Snuggle commented Dec 28, 2018

As for testing, I'll be happy to test uhh... Tomorrow if I have free time and can remember!

@matchai
Copy link
Owner Author

matchai commented Dec 28, 2018

Thank you @Snuggle! Hope you had some enjoyable time off. 😎🌴
I've also been taking it easy the last couple weeks.

Sounds perfect! No pressure if you don't get around to it right away. 😄

@matchai
Copy link
Owner Author

matchai commented Jan 1, 2019 •
edited
Loading

@Snuggle Just a friendly reminder that this PR needs testing if you've got some free time. 😄
(Happy new year! 🎉)

@Snuggle
Copy link
Collaborator

Snuggle commented Jan 9, 2019 •
edited
Loading

After testing, my prompt uses my wireless mouse's battery and reports 0%. No errors though! 🥂

screenshot from 2019-01-09 09-25-27

Ubuntu 18.10 on a generic Samsung laptop and a Logitech MX Master (1st gen.) wireless mouse.

@Snuggle
Copy link
Collaborator

Snuggle commented Jan 9, 2019 •
edited
Loading

I can't remember where the conversation was, either on this repo or Spaceship, but there needs to be some logic in place to ignore a 0% battery level. Perhaps to return the first non-zero battery level.

@Snuggle
Copy link
Collaborator

Snuggle commented Jan 9, 2019

This pull request does fix the following bug, though and is better than master. I'll try and join in @salmanulfarzy's discussion on how to handle multiple batteries, should be interesting. 🙂

screenshot from 2019-01-09 09-33-29

@matchai
Copy link
Owner Author

matchai commented Jan 9, 2019

Yep! This is by no means a perfect solution. Just a better one for the time being. 😃
Thanks for taking the time to manually test it! 🍻

@salmanulfarzy
Copy link
Contributor

salmanulfarzy commented Jan 9, 2019 •
edited
Loading

Did you accidentally revert the original changes (2e2aa99) with this merge commit (2c7bd67) or was it intentional ? Seems like the only change now is comment removal 🤷‍♂️

@matchai
Copy link
Owner Author

matchai commented Jan 9, 2019

Oh whoops. That was definitely a slip-up! 😱
Solid catch @salmanulfarzy!

@matchai matchai merged commit 8fa713b into master Jan 9, 2019
@matchai matchai deleted the fix-124 branch January 9, 2019 16:10
matchai pushed a commit that referenced this pull request Jan 9, 2019
@semantic-release-bot
chore(release): 2.0.1 [skip ci]
c9656fa 
## [2.0.1](v2.0.0...v2.0.1) (2019-01-09)

### Bug Fixes

* have ACPI use only the first battery value ([#126](#126)) ([8fa713b](8fa713b))
@matchai
Copy link
Owner Author

matchai commented Jan 9, 2019

🎉 This PR is included in version 2.0.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tiffafoo tiffafoo tiffafoo approved these changes

@Snuggle Snuggle Snuggle approved these changes

Assignees
No one assigned
Labels
Status: Released Type: Bug Fix an existing bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Battery section: ACPI use only first battery value
4 participants
@matchai @Snuggle @salmanulfarzy @tiffafoo