-
Notifications
You must be signed in to change notification settings - Fork 986
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
Diagnose up-to 10 batteries #1711
Diagnose up-to 10 batteries #1711
Conversation
6b5bb8a
to
abd53a4
Compare
Tested to work in hardware, but I do not like the third commit, for obvious reasons. |
@vooon can you help me out here? |
I think emplace should construct object, so you should only provide constructor arguments. Also for single-arg it's recommended to use Also can we use |
abd53a4
to
4cf4f02
Compare
Thanks, but the error is still there:
|
I'll try to implement my thoughts, but that might take more than weekend. |
Thanks @vooon |
e08cfba
to
ab11040
Compare
Nevermind, I have fixed and tested it to run fine. I have multiple batteries with running mutexes now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thanks!
52af6dc
to
56b695f
Compare
No clue why CI is complaining on this one. |
Uses as many battery monitors as the user specified in min_voltage parameter. Add myself as a contributor, this is not my first patch to this file
56b695f
to
866e807
Compare
866e807
to
92b40f5
Compare
OK, CI fixed :) |
I am open to suggestions to avoid the third commit