OSX: implement sensors_battery #1177

wiggin15 · 2017-11-19T08:54:35Z

No description provided.

giampaolo

Cool stuff. Some comments inline. Also another thing. In the future we'll probably want to make sensors_battery return info about multiple batteries as a list. Are these APIs able to do that?

Also, it would be good to have a specific test comparing results with a CLI tool. See here:
http://osxdaily.com/2015/12/10/get-mac-battery-life-info-command-line-os-x/

giampaolo · 2017-11-19T08:59:29Z

psutil/_psutil_osx.c

+
+    if (!power_info) {
+        PyErr_SetString(PyExc_RuntimeError,
+            "Failed to get power sources info");


I would change the error message to "IOPSCopyPowerSourcesInfo() syscall failed"

I was following the style of "psutil_disk_io_counters" where errors for "IO*" functions set strings like "unable to get the disk properties" - but I will change this.

OK, I will change those ones then. In general I will probably want to apply this paradigm everywhere.

giampaolo · 2017-11-19T09:00:17Z

psutil/_psutil_osx.c

+    power_sources_list = IOPSCopyPowerSourcesList(power_info);
+    if (!power_sources_list) {
+        PyErr_SetString(PyExc_RuntimeError,
+            "Failed to get power sources list");


I would change the err message to "IOPSCopyPowerSourcesList() syscall failed"

giampaolo · 2017-11-19T09:01:57Z

psutil/_psutil_osx.c

+        time_to_empty = -1;
+    }
+
+    py_retlist = Py_BuildValue("Iii",


I think Py_BuildValue("Iii" produces a tuple, not a list.

I'm following the style of the rest of psutil. the name py_retlist is used for tuples as well as lists everywhere in the code.
There is no variable currently named "py_rettuple" or "py_retobj" etc. What name would you suggest?

I'm following the style of the rest of psutil. the name py_retlist is used for tuples as well as lists everywhere in the code.

Really? I didn't realize that. Anyway, I suggest py_tuple in this case.

giampaolo · 2017-11-19T09:06:38Z

psutil/_psutil_osx.c

+    /* Should only get one source. But in practice, check for > 0 sources */
+    if (!CFArrayGetCount(power_sources_list)) {
+        PyErr_SetString(PyExc_RuntimeError,
+            "Power sources list is empty");


I checkout out your branch and bumped into this error on my virtualized OSX box.
It seems this happens where there is not battery so we probably have to return None here.

Already done :)

wiggin15 · 2017-11-19T10:48:01Z

Are these APIs able to do that?

Since the API returns "PowerSourcesList", I assume this can include more than one source, but I will have no way to test this.

giampaolo · 2017-11-19T10:50:15Z

Since the API returns "PowerSourcesList", I assume this can include more than one source, but I will have no way to test this.

OK, let's ignore this for now. But just FYI my plan is to have:

sensors_battery(all=True) -> [ntuple(...), ...]

wiggin15 · 2017-11-19T12:42:22Z

Updated commit

giampaolo · 2017-11-19T18:04:31Z

psutil/_psosx.py

+        percent, minsleft, power_plugged = cext.sensors_battery()
+    except NotImplementedError:
+        # no power source - return None according to interface
+        return None


nice solution, I like it

giampaolo · 2017-11-19T18:05:29Z

psutil/tests/__init__.py

@@ -70,7 +70,7 @@
    'VERBOSITY',
    "HAS_CPU_AFFINITY", "HAS_CPU_FREQ", "HAS_ENVIRON", "HAS_PROC_IO_COUNTERS",
    "HAS_IONICE", "HAS_MEMORY_MAPS", "HAS_PROC_CPU_NUM", "HAS_RLIMIT",
-    "HAS_SENSORS_BATTERY", "HAS_BATTERY""HAS_SENSORS_FANS",
+    "HAS_SENSORS_BATTERY", "HAS_BATTERY", "HAS_SENSORS_FANS",


giampaolo · 2017-11-19T18:05:50Z

psutil/tests/test_osx.py

+        power_plugged = drawing_from == "AC Power"
+        psutil_result = psutil.sensors_battery()
+        self.assertEqual(psutil_result.power_plugged, power_plugged)
+        self.assertEqual(psutil_result.percent, int(percent))


@wiggin15

* 'pslisten' of github.com:nlevitt/psutil: (922 commits) Update INSTALL.rst Pass python_requires argument to setuptools (giampaolo#1208) giampaolo#1152: fix doc to mention CLI command necessary to enable disk_io_counters() on win pre release pre release pre release pre-release fix giampaolo#1201: document that timeout kwarg is expressed in seconds Add mount points to disk_partitions() in Windows (giampaolo#775) (giampaolo#1192) add test for cpu_affinity what a stupid bug! (giampaolo#1190) update doc pre release pre-release; also get rid of PSUTIL_DEBUG doc instructions (it's kinda useless for the user after all) Use FutureWarning instead of DeprecationWarning (giampaolo#1188) fix test refactor environ() test Fix OSX pid 0 bug (giampaolo#1187) change assert in test refactor Process.__repr__ Faster Process.children(recursive=True) (giampaolo#1186) Speedup Process.children() (giampaolo#1185) update doc update HISTORY fix giampaolo#1179 / linux / cmdline: handle processes erroneously overwriting /proc/pid/cmdline by using spaces instead of null bytes as args separator set x bit to test_aix.py fix giampaolo#1181: raise AD if task_for_pid() fails with 5 and errno == ENOENT fix posix failure Arguments for NoSuchProcess and AccessDenied for the C ext (giampaolo#1180) fix travis failure https://travis-ci.org/giampaolo/psutil/jobs/306424509 be smarter in searching python exe do not test platf specific modules on wheelhouse try to fix travis failure fix travis failures try to use PYTHON_EXE instead of sys.executable giampaolo#1177: give credits to @wiggin15 OSX: implement sensors_battery (giampaolo#1177) improve error msg for old windows systems giampaolo#811 add debug messages do not mention apt-get as method of installation as it's not recommended syntax highlight in doc files syntax highlight in doc files fix doc indentation 1173 debug mode (giampaolo#1176) code style update MANIFEST giampaolo#1174: use TimeoutExpired in wait_pid() sort imports by name Move exceptions to separate file (giampaolo#1174) appveyor: enable python warnings when running tests refactor winmake.py use a C global variable to figure out whether we're in testing mode fix unicode err define a setup() function which is called on import by all C modules move PyUnicode compt fun definition up in the file rename C func re-enable test on appveyor; remove unused C code refactor PSUTIL_TESTING C APIs inspect PSUTIL_TESTING env var from C again giampaolo#1152: (DeviceIOControl), skip disk on ERROR_INVALID_FUNCTION and ERROR_NOT_SUPPORTED giampaolo#1152 / win / disk_io_counters(): DeviceIOControl errors were ignored; che return value and retry call on ERROR_INSUFFICIENT_BUFFER upgrade dist cmds change make cmds disable IPv6 tests if IPv6 is not supported travis / OSX: run py 3.6 instead of 3.4 fix giampaolo#1169: (Linux) users() hostname returns username instead update README, bump up version get rid of PSUTIL_TESTING env var: it must be necessarily set from cmdline, hence 'python -m psutil.tests' won't work out of the box try to set PSUTIL_TESTING env var from python before failing skip cpu_freq tests if not available (giampaolo#1170) update doc pre-release giampaolo#1053: drop python 3.3 support try to fix appveyor failure; also refactor generate_manifest.py giampaolo#1167 give CREDITS to @matray Including non-unicast packets in packet count calculation (giampaolo#1167) fix giampaolo#1166 (doc mistake) provide a 'make help' command ifconfig.py humanize bytes try to limit false positives on appveyor/windows reap_children() in a finally block in order to limit false positives unicode tests: use different name for test dir fix failure on osx/travis update Makefile fix test giampaolo#1164 give CREDITS to @wiggin15 AIX: implement num_ctx_switches (giampaolo#1164) use new PYTHON_EXE improve logic to determine python exe location add DEVNOTES file; move TODO.aix into _psutil_aix.c Fix test_emulate_energy_full_not_avail (giampaolo#1163) update README try to limit occasional appveyor failure Remove trove classifiers for untested and unsupported platforms (giampaolo#1162) Fix giampaolo#1154: remove 'threads' method on older AIX (giampaolo#1156) give CREDITS to @adpag for giampaolo#1159, giampaolo#1160 and giampaolo#1161 Fix test asserts due to leftover test subprocesses (giampaolo#1161) Fix network tests for newer ifconfig versions. (giampaolo#1160) Fix pre-commit hook for python 3.x. (giampaolo#1159) revert last commit ...

wiggin15 force-pushed the osx_sensors_battery branch from a41457b to d3b9f14 Compare November 19, 2017 08:55

giampaolo requested changes Nov 19, 2017

View reviewed changes

wiggin15 force-pushed the osx_sensors_battery branch from d3b9f14 to 6d36e65 Compare November 19, 2017 09:26

OSX: implement sensors_battery

b1b8d27

wiggin15 force-pushed the osx_sensors_battery branch from 6d36e65 to b1b8d27 Compare November 19, 2017 12:41

giampaolo reviewed Nov 19, 2017

View reviewed changes

giampaolo merged commit 3a3598e into giampaolo:master Nov 19, 2017

giampaolo added a commit that referenced this pull request Nov 22, 2017

#1177: give credits to @wiggin15

b0c1b9f

giampaolo added the macos label Nov 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OSX: implement sensors_battery #1177

OSX: implement sensors_battery #1177

wiggin15 commented Nov 19, 2017

giampaolo left a comment

giampaolo Nov 19, 2017

wiggin15 Nov 19, 2017

giampaolo Nov 19, 2017

giampaolo Nov 19, 2017

giampaolo Nov 19, 2017

wiggin15 Nov 19, 2017

giampaolo Nov 19, 2017

giampaolo Nov 19, 2017

wiggin15 Nov 19, 2017

wiggin15 commented Nov 19, 2017

giampaolo commented Nov 19, 2017

wiggin15 commented Nov 19, 2017

giampaolo Nov 19, 2017

giampaolo Nov 19, 2017

giampaolo Nov 19, 2017

OSX: implement sensors_battery #1177

OSX: implement sensors_battery #1177

Conversation

wiggin15 commented Nov 19, 2017

giampaolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wiggin15 commented Nov 19, 2017

giampaolo commented Nov 19, 2017

wiggin15 commented Nov 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment