[1.1.x] Add new capability to report if Thermal Protection is enabled#10465
[1.1.x] Add new capability to report if Thermal Protection is enabled#10465thinkyhead merged 2 commits intoMarlinFirmware:bugfix-1.1.xfrom andrivet:add_cap_thermal_protection
Conversation
thinkyhead
force-pushed
the
bugfix-1.1.x
branch
3 times, most recently
from
0bebc14 to
0c1be96
Compare
|
Capabilities are not so much for announcing features, but to tell the host that it can or should modify its behavior to accommodate the feature. Thermal protection doesn't change any behavior w/r/t hosts, nor does it add any G-codes. @MarlinFirmware/host-software-team — Will this be useful to your efforts? |
|
While I agree that this is less of a capability relevant for the protocol layer and more a case of making the blackbox that is the firmware less blackboxy, it certainly would help a lot in identifying printers with unsafe configuration down the road (once this migrates a bit through the various forks) and educate/warn users accordingly. The constant flow of "Oops, an Anet A8 caught fire" posts on social media show that there's not a whole lot of awareness on this. In fact, I suggested to @andrivet to send a PR upstream when he approached me about adding this report to his own fork and if he hadn't filed this PR, I would have. |
|
@thinkyhead Even if I understand your point of view, I think that in some cases, this PR fit the "tell the host that it can or should modify its behavior to accommodate the feature". In case of my firmware (ADVi3++), the user has the possibility to enable or disable the Thermal Runaway Protection (it is ON by default). So the new capability allow the host (such as OctoPrint) accommodate to this. But it is true it is a corner case. |
In response to `M115` the firmware reports if Thermal Protection is enabled (value 1) or not (value 0). This information can be used by software such as OctoPrint (more precisely the Printer Safety Check plugin) to warn users when Thermal Protection is not enabled in the firmware.
|
Naturally. My post doesn't reflect my own point-of-view, per se. Part of my role is to state counter-examples, consider potential points-of-view, and solicit feedback while keeping my own view in suspension (as far as that is possible). Since this is obviously useful going forward, I support it. But note that it could be a year or more before vendors start shipping machines with Marlin 1.1.9, and if they fail to enable the capabilities report, or if they alter the report to suppress the host warning, then hosts and users will still be in the dark. So, if I may play the Devil's advocate again… |
|
In my mind "your point of view" means the point of view of Marlin Team, the point of view of your role. For your question ("Should hosts report "WARNING!..."), for me the answer is clearly no. The host has to report WARNING only if the capability is 0. Not if the capability is not there. |
|
…a putative point of view floating in the aether… |
Based on #10465 In response to `M115` the firmware reports if Thermal Protection is enabled (1) or not (0). This information can be used by software such as OctoPrint (more precisely the Printer Safety Check plugin) to warn users when Thermal Protection is not enabled in the firmware. Co-Authored-By: andrivet <sebastien@advtools.com>
|
If I see this correctly, you now start mixing capability with configuration. On the other side thermal protection can change. In repetier-firmware you can disable it if required and @andrivet also made it changeable if I understood him correctly. In the sense of capability report it would make more sense that this only announces that a certain set of M codes is supported to query if it is enabled and maybe even to enable/disable them so hosts can offer a switch in case they see it is enabled. For configurations there is M360 in repetier-firmware that reports its configuration. I guess Marlin has a similar command. Then it would make sense to show a warning in host that it currently disabled knowing that firmware allows enabling. This function is surely an important point as it is a security function. And having a security checklist in hosts might be a good thing to have in future. One problem are as always old firmwares. And we all should know that there are users not daring to update firmwares and vendors never updating, while others offer updates. So each check we want to implement has four states:
Not having a capability is 1). Cap 0 is 2) and 3 and 4 should be cap 1 determinable and changeable by commands. Knowing many vendors they also might want to enforce 4) removing the changeable option as it is their reputation that goes when users can disable safety features. |
|
When I made this PR, my intention was not to treat this as a configuration (something that can change often), but as a capability (something the host can check at startup). It is true that the user can change it while running, but we can assume that the user will not change this again and again. It is enough to warn her about the danger when booting. Again, I agree that this is a corner case. |
Yes, capabilities will announce to the host, for example, that there's a light on the printer that can be turned on and off. The light is assumed by the host to be there if the capability announces it, so the host knows it can always turn it on and off.
There's no command in Marlin to announce configuration. It would be a prohibitively large number of strings to fit on modest boards such as the common Melzi.
A fifth state is missing: "Always enabled."
No. The thermal runaway values set in the configuration are fixed and cannot be altered by G-code. Once thermal protection is enabled in Marlin and flashed, it is always turned on for all heating operations and cannot be disabled by a host. That is as it should be. I realize you're not proposing an extension to |
No. But if the host reports
Yep, agreed. Based on past observations of vendor firmware though, I'd not be surprised if they'd happily disable thermal runaway protection for better stability with sub par sensors but leave this report in. Least effort approach, as always ;) |
|
@repetier is there documentation available for the |
|
@thinkyhead Capabilities are fixed. 0 = know it bot do not support, 1 = support. If it is clearly stated in the description on http://reprap.org/wiki/Firmware_Capabilities_Protocol there is problem adding a 2 as e.g. switchable through commands xy. Speaking of the wiki, could you add the new capabilities so we host developers still have a 1 place checkup page to see if we catched all or if there are new ones we should start supporting. @foosel At the moment there is no report of this setting except if you query eeprom and that may change description/position especially if V2 is ready. I think I will switch over to report it as capability and make it non changeable once the exact format is known. Regarding testing firmware settings you can always have a look at repetier.xml in firmware folder in server installation. It shows how we query for settings of repetier-firmware. Here as copy the current config with it's regular expressions: |
Done! |
* Repetier firmware versions prior to 0.92 don't have thermal runaway/
decouple detection, so we always warn about those
* If the firmware supports the THERMAL_PROTECTION capability report
but it's reported as disabled, we also warn about that (see also
MarlinFirmware/Marlin#10465)
4 participants
Description
This code add a new capability to report if Thermal Protection is enabled (value 1) or not (value 0). Capabilities are reported by the M115 command. Thermal Protection is a feature of Marlin and is enabled with
THERMAL_PROTECTION_HOTENDSandTHERMAL_PROTECTION_BEDmacros.The idea behind this new capability is to increase the awareness of users regarding Thermal Protection and the danger to disable it. Thanks to this new capability, software such as OctoPrint (more precisely the Printer Safety Check plugin) will be able to warn users when Thermal Protection is not enabled in the firmware.
Benefits
It is a very minor and simple modification and it can increase the awareness of users regarding Thermal Protection and the danger to disable it.
Related Issues