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

Battery notifications dependent on showing of menubar symbol #240

Open
flutterrausch opened this issue Nov 1, 2020 · 5 comments
Open

Battery notifications dependent on showing of menubar symbol #240

flutterrausch opened this issue Nov 1, 2020 · 5 comments
Labels
bug Something isn't working

Comments

@flutterrausch
Copy link

I only get low/high battery warning notifications, if showing of menubar symbol is "on". It would be useful to get those warnings without the need to clutter the menubar.

To Reproduce
Open iGlance Window/Batter. Set "show low battery notification at" to 1% lower of current battery state. Wait.
Do this with battery menubar icon on/off.

Expected behavior
Notification should ALWAYS appear.

  • MacOS Version: 10.15.7
  • Version of the app: 2.1

Log shouldn't be necessary. Or just ask.

Thanks!

@issue-label-bot
Copy link

Issue Label Bot is not confident enough to auto-label this issue. See dashboard for more details.

@Moneypulation
Copy link
Member

It seems to be a logical bug in the code. Here we return from the update function if the menu bar icon is not visible. But therefore we also skip the battery notification functions in L71 and L74.

A fix would be to rearrange the code in the function. For instance:

  1. update struct variables
  2. call battery notification functions
  3. check if menu bar item is visible. if yes, call menu bar item update functions, else return

@Moneypulation Moneypulation added the bug Something isn't working label Nov 1, 2020
@flutterrausch
Copy link
Author

flutterrausch commented Nov 1, 2020

Exactly:

func update() {
    self.statusItem.isVisible = AppDelegate.userSettings.settings.battery.showBatteryMenuBarItem

    // get the current charge
    let currentCharge = AppDelegate.systemInfo.battery.getCharge()

    // notify the user about the capacity of the battery if necessary
    if AppDelegate.userSettings.settings.battery.lowBatteryNotification.notifyUser {
        deliverLowBatteryNotification(currentCharge: currentCharge)
    }
    if AppDelegate.userSettings.settings.battery.highBatteryNotification.notifyUser {
        deliverHighBatteryNotification(currentCharge: currentCharge)
    }


    // do not update invisible icon
    if !self.statusItem.isVisible {
        return
    }

    // get the state of the internal battery
    let batteryState = AppDelegate.systemInfo.battery.getInternalBatteryState()
    // get whether the battery is on AC
    let isOnAC = AppDelegate.systemInfo.battery.isOnAC()
    // get whether the battery is charging
    let isCharging = AppDelegate.systemInfo.battery.isCharging()
    // get whether the battery is charged
    let isCharged = AppDelegate.systemInfo.battery.isFullyCharged()

    // update the menu bar icon and the menu of the menu bar item
    updateMenuBarIcon(currentCharge: currentCharge, isOnAC: isOnAC, isCharging: isCharging, isCharged: isCharged, batteryState: batteryState)
    updateMenuBarMenu(currentCharge: currentCharge, batteryState: batteryState)

    self.lastBatteryCharge = currentCharge
}

@D0miH
Copy link
Member

D0miH commented Nov 1, 2020

Hey @flutterrausch,
thanks for the bug report. Yes, this is indeed a bug and was not intended.
It would be nice if you could open a new pull request with the fix you demonstrated above.

@flutterrausch
Copy link
Author

flutterrausch commented Nov 1, 2020

Will try. Is there a potential problem with Xcode 12.0.1 and carthage?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants