Emit warning when current/voltage limits are near zero #76

Merged
sb10q merged 1 commits from esavkin/thermostat:72-warning-on-zeroed-limits into master 2023-03-23 17:01:29 +08:00
Owner

Closes #72

>>> from pytec.pytec.client import Client
>>> Client() # pwm 1 max_v 0.1
<pytec.pytec.client.Client object at 0x7f376d6e59d0>
>>> Client() # pwm 1 max_v 0
WARNING:root:`max_v` limit is set to zero at channel 1
<pytec.pytec.client.Client object at 0x7f376d723c70>
Closes #72 ```py >>> from pytec.pytec.client import Client >>> Client() # pwm 1 max_v 0.1 <pytec.pytec.client.Client object at 0x7f376d6e59d0> >>> Client() # pwm 1 max_v 0 WARNING:root:`max_v` limit is set to zero at channel 1 <pytec.pytec.client.Client object at 0x7f376d723c70> ```
esavkin requested review from sb10q 2023-01-18 15:16:23 +08:00
sb10q requested changes 2023-01-23 13:28:30 +08:00
@ -11,0 +17,4 @@
pwm_report = self.get_pwm()
for pwm_channel in pwm_report:
for limit in LIMITS:
if isclose(pwm_channel[limit]["value"], 0.0, rel_tol=1e-6):
Owner

rel_tol is not the correct one to use here. Please read the Python manual carefully or just write the formula explicitly, I don't think there's much of a point to math.isclose.

Did you even test this?

>>> isclose(0.00000000000000001, 0.0, rel_tol=1e-6)
False
``rel_tol`` is not the correct one to use here. Please read the Python manual carefully or just write the formula explicitly, I don't think there's much of a point to ``math.isclose``. Did you even test this? ``` >>> isclose(0.00000000000000001, 0.0, rel_tol=1e-6) False ```
esavkin force-pushed 72-warning-on-zeroed-limits from 6c601f8423 to 1735e6c030 2023-01-26 10:33:44 +08:00 Compare
sb10q reviewed 2023-01-26 10:55:58 +08:00
@ -11,0 +17,4 @@
pwm_report = self.get_pwm()
for pwm_channel in pwm_report:
for limit in LIMITS:
if isclose(pwm_channel[limit]["value"], 0.0, abs_tol=1e-6):
Owner

Isn't just == 0.0 sufficient for the purpose explained in the Issue?

Isn't just ``== 0.0`` sufficient for the purpose explained in the Issue?
Author
Owner

Looks like it's quite safe to do so

Looks like it's quite safe to do so
esavkin force-pushed 72-warning-on-zeroed-limits from 1735e6c030 to f9f526d7fe 2023-01-26 11:50:20 +08:00 Compare
sb10q reviewed 2023-02-08 19:24:45 +08:00
@ -11,0 +17,4 @@
for pwm_channel in pwm_report:
for limit in LIMITS:
if pwm_channel[limit]["value"] == 0.0:
logging.warning("`{}` limit is set to zero at channel {}".format(limit, pwm_channel["channel"]))
Owner

on channel

on channel
esavkin force-pushed 72-warning-on-zeroed-limits from f9f526d7fe to 7b35932d36 2023-02-16 10:27:28 +08:00 Compare
esavkin force-pushed 72-warning-on-zeroed-limits from 7b35932d36 to e3e3237d2f 2023-03-23 16:58:25 +08:00 Compare
sb10q merged commit e3e3237d2f into master 2023-03-23 17:01:29 +08:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/thermostat#76
No description provided.