iglance / iGlance

Free system monitor for OSX and macOS. See all system information at a glance in the menu bar.
https://iglance.github.io
GNU General Public License v3.0
2.43k stars 123 forks source link

Battery notifications dependent on showing of menubar symbol #240

Open flutterrausch opened 4 years ago

flutterrausch commented 4 years ago

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.

Log shouldn't be necessary. Or just ask.

Thanks!

issue-label-bot[bot] commented 4 years ago

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

Moneypulation commented 4 years ago

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
flutterrausch commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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