From 0d8fdfae26a46df5aa317933fce3373410dbbbd9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= <osd@foosel.net>
Date: Mon, 17 Oct 2016 10:27:06 +0200
Subject: [PATCH] Trust Tc over T when parsing temps from the firmware

---
 src/octoprint/util/comm.py      | 36 ++++++++++++++++-----------------
 tests/util/test_comm_helpers.py | 16 +++++++++++----
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/src/octoprint/util/comm.py b/src/octoprint/util/comm.py
index a88a0b3b..b796e4c1 100644
--- a/src/octoprint/util/comm.py
+++ b/src/octoprint/util/comm.py
@@ -2654,8 +2654,8 @@ def canonicalize_temperatures(parsed, current):
 
 	  * If ``T`` is not included with the reported extruders, return
 	  * If more than just ``T`` is reported:
-	    * If both ``T`` and ``T0`` are reported set ``Tc`` to ``T``, remove
-	      ``T`` from the result.
+	    * If both ``T`` and ``T0`` are reported, remove ``T`` from
+	      the result.
 	    * Else set ``T0`` to ``T`` and delete ``T`` (Smoothie extra).
 	  * If only ``T`` is reported, set ``Tc`` to ``T`` and delete ``T``
 	  * return
@@ -2679,23 +2679,21 @@ def canonicalize_temperatures(parsed, current):
 
 	if len(reported_extruders) > 1:
 		if "T0" in reported_extruders:
-			# Both T and T0 are present, so T contains the current
-			# extruder's temperature, e.g. for current_tool == 1:
+			# Both T and T0 are present, let's check if Tc is too.
+			# If it is, we just throw away T (it's redundant). It
+			# it isn't, we first copy T to Tc, then throw T away.
 			#
-			#     T:<T1> T0:<T0> T2:<T2> ... B:<B>
-			#
-			# becomes
-			#
-			#     T0:<T1> T1:<T1> T2:<T2> ... B:<B>
-			#
-			# Same goes if Tc is already present, it will be overwritten:
-			#
-			#     T:<T1> T0:<T0> T1:<T1> T2:<T2> ... B:<B>
-			#
-			# becomes
-			#
-			#     T0:<T0> T1:<T1> T2:<T2> ... B:<B>
-			result[current_tool_key] = result["T"]
+			# The easier construct would be to always overwrite Tc
+			# with T and throw away T, but that assumes that if
+			# both are present, T has the same value as Tc. That
+			# might not necessarily be the case (weird firmware)
+			# so we err on the side of caution here and trust Tc
+			# over T.
+			if current_tool_key not in reported_extruders:
+				# T and T0 are present, but Tc is missing - copy
+				# T to Tc
+				result[current_tool_key] = result["T"]
+			# throw away T, it's redundant (now)
 			del result["T"]
 		else:
 			# So T is there, but T0 isn't. That looks like Smoothieware which
@@ -2719,7 +2717,7 @@ def canonicalize_temperatures(parsed, current):
 		#     reports the current tool and bed
 		#
 		# In both cases it is however safe to just move our T over
-		# to T<current> in the parsed data, current should always stay
+		# to Tc in the parsed data, current should always stay
 		# 0 for single extruder printers. E.g. for current_tool == 1:
 		#
 		#     T:<T1>
diff --git a/tests/util/test_comm_helpers.py b/tests/util/test_comm_helpers.py
index 93a00d16..a8870c28 100644
--- a/tests/util/test_comm_helpers.py
+++ b/tests/util/test_comm_helpers.py
@@ -213,15 +213,23 @@ class TestCommHelpers(unittest.TestCase):
 		self.assertEqual(expected_max, maxtool)
 
 	@data(
+		# T => T0
 		(dict(T=(23.0,None)), 0, dict(T0=(23.0, None))),
+
+		# T => T1
 		(dict(T=(23.0,None)), 1, dict(T1=(23.0, None))),
+
+		# T and Tn present => Tn wins
 		(dict(T=(23.0, None), T0=(23.0, None), T1=(42.0, None)), 0, dict(T0=(23.0, None), T1=(42.0, None))),
 		(dict(T=(42.0, None), T0=(23.0, None), T1=(42.0, None)), 1, dict(T0=(23.0, None), T1=(42.0, None))),
-		(dict(T=(21.0, None), T0=(23.0, None), T1=(42.0, None)), 0, dict(T0=(21.0, None), T1=(42.0, None))),
-		(dict(T=(41.0, None), T0=(23.0, None), T1=(42.0, None)), 1, dict(T0=(23.0, None), T1=(41.0, None))),
-		(dict(T=(23.0, None), T1=(42.0, None)), 1, dict(T0=(23.0, None), T1=(42.0, None))),
-		(dict(T0=(23.0, None), T1=(42.0, None)), 1, dict(T0=(23.0, None), T1=(42.0, None)))
+		(dict(T=(21.0, None), T0=(23.0, None), T1=(42.0, None)), 0, dict(T0=(23.0, None), T1=(42.0, None))),
+		(dict(T=(41.0, None), T0=(23.0, None), T1=(42.0, None)), 1, dict(T0=(23.0, None), T1=(42.0, None))),
 
+		# T and no T0 => Smoothieware, T = T0
+		(dict(T=(23.0, None), T1=(42.0, None)), 1, dict(T0=(23.0, None), T1=(42.0, None))),
+
+		# no T => as-is
+		(dict(T0=(23.0, None), T1=(42.0, None)), 1, dict(T0=(23.0, None), T1=(42.0, None)))
 	)
 	@unpack
 	def test_canonicalize_temperatures(self, parsed, current, expected):