-
-
Notifications
You must be signed in to change notification settings - Fork 254
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
Enable PID ON / PID OFF Alarm Actions for Arduino/TC4 #205
Comments
Dear James, thanks for this detailed report. Instead of changing the pre-conditions in the cases you indicate, wouldn't it be more consistent to just enable the PID features and show the "Control" button if the "Control" flag is ticked also for the case the TC4 PID firmware is selected? That way a user could run the TC4 PID firmware and still deactivate the PID features (eg. for testing something) with one tick. It seems that if the "Control" flag is ticked the PID alarm actions work as expected as are the keyboard shortcuts. Do I overlook something? |
Hi Marko, yes that sounds good. It doesn't look like there are any unintentional side effects of having the Control Flag ticked too, as the critical code is guarded with if/elif statements to catch the TC4 PID actions first. As you suggest, for consistency you could then hide the Control Button if the Control Flag is unticked for the TC4 PID as well: line 12939-12947:
This will then also match the description here:
That just leaves the FUJI PID to have a think about, as it is handled irrespective of the Control Flag :) |
…d to tick the Control flag to activate the ´Control` button and the PID features also for the case of the TC4 with PID Firmware as for the Fuji PIDs (closes Issue #205)
Thanks for your comment. Hope I did it correct now in v1.3.1 |
Expected Behavior
Alarm actions PID ON and PID OFF should enable / disable hardware PID on Arduino/TC4 platform. RampSoak ON / OFF may also be affected.
Actual Behavior
PID state on Aruidno/TC4 does not change when alarm activates.
Steps to Reproduce the Problem
Alarm handlers only work if aw.qmc.Controlbuttonflag is set (SW only?). aw.qmc.Controlbuttonflag case is handled in pidOn() / pidOff() functions so this condition may be able to be removed from the alarm handlers to keep them generalized for all PID types.
main.py : 2802-2827
Might be worth checking if this pattern occurs elsewhere in the code (e.g. key press handlers: ln14861, ln14874, ln14881?).
Specifications
The text was updated successfully, but these errors were encountered: