General bugfixes

[thdrmrphy]

This PR:

• adds validation, based on @ksgubanov's work on #228, but slightly fixed
• fixes eternal loops where disable_discharging would disable_charging, even when called from within enable_charging
• fixes some MagSafe light stuff
• adds more verbose logging - this can probably be fixed; it's quite messy, but it's more descriptive than before
• takes accuracy code from #241 and seems to work properly ¯\_(ツ)_/¯
• still has issues, but doesn't add any new issues that I know of.

I'm very much open to suggestions. This PR kind of throws everything at trying to fix bugs and so may have made some things a bit messier than they used to be. Having said that, I've been stress-testing this code daily for a while and I've not run into the original issues any longer.

Hope it helps!

[jkellerer]

@thdrmrphy thanks a lot for the efforts in improving the app. However the change in #244 broke one of the main features of the app, the ability to disable the charge limit and charge the battery to 100% (from the UI). Because when you disable it, "stop" is the $setting and it ends up in the disable_charge branch (... what also looks strange is that battery_percentage is only updated after use but that doesn't change anything for that matter).

This PR is likely fixing this part, but I'm not convinced that disable_discharge should do anything else than setting the state with smc (and be symmetric to enable_discharge), especially when it is used at many locations that expect its original behaviour.

I'd consider a new method if the same sort of logic (disabling discharge and maintaining the state) is needed at multiple locations.

What should not happen, however is that the primary function of the app (limit the charge and allow to toggle the limit from the UI) breaks.

[Chr1s70ph]

Is there anything I can contribute to this MR? I would greatly appreciate the fixes and even tried fixing them myself before i found this

[thdrmrphy]

Hey @jkellerer thanks for taking a look at this. It's been a bit since I really took a look at this code so forgive me if I'm missing something. I do think this PR fixes that issue (I'm not convinced the issue was caused by #244 but I don't think I helped it by adding the extra logic) and from what I remember, disable_discharging had to do all that new logic because if one disabled discharging by unticking the option called "Allow force-discharging" (which also had some Electron issues all along but that's a different matter) then the app would have ignored the last-used setting. As far as I'm concerned, the app should do everything it does and then have a layer on top that enables/disables force-discharging, which otherwise doesn't affect the app's function.

Also, I have no idea why I put battery_percentage=$(get_battery_percentage) at the end. No clue. If someone can work out what should be happening that would be awesome, I assume it should just be at the start but it's been a while and I don't remember.

Having said all that there's a good chance I'm missing something and whilst I haven't had the issue you're describing with the UI ("it works on my machine") the app currently needs a lot of reworking and this PR is definitely not ready to merge. @Chr1s70ph thank you so much for the help, to anyone wanting to contribute I think the next steps that we can take here are probably to identify and document the issues that @jkellerer mentioned (as well as any others that pop up) including the relevant logs from the app and then we can try to find out where it's going wrong. Also, error handling (or lack thereof) and variable validation are major issues here so if perhaps we can try to implement checks at every possible point (so that we don't keep getting issues where the app is trying to work out whether a string or a numerical value is higher). From memory there might be some other open PRs to help with this.

I'll keep an eye on this and try to contribute where I can but thanks for the input everyone and I hope this gets us somewhere more stable.

[ksgubanov]

closes #210