Support remote M308, for thermistors only so far

8 files changed, 341 insertions, 113 deletions

diff --git a/src/Heating/Heat.cpp b/src/Heating/Heat.cpp
index 0e8dbc29..9723a5af 100644
--- a/src/Heating/Heat.cpp
+++ b/src/Heating/Heat.cpp
@@ -41,6 +41,10 @@ Licence: GPL
 # include "CAN/CanInterface.h"
 #endif
 
+#if SUPPORT_REMOTE_COMMANDS
+# include <CanMessageGenericParser.h>
+#endif
+
 #ifdef DUET3_ATE
 # include <Duet3Ate.h>
 #endif
@@ -1227,4 +1231,60 @@ void Heat::ProcessRemoteHeatersReport(CanAddress src, const CanMessageHeatersSta
 
 #endif
 
+#if SUPPORT_REMOTE_COMMANDS
+
+GCodeResult Heat::EutProcessM308(const CanMessageGeneric& msg, const StringRef& reply) noexcept
+{
+	CanMessageGenericParser parser(msg, M308NewParams);
+	uint16_t sensorNum;
+	if (parser.GetUintParam('S', sensorNum))
+	{
+		if (sensorNum < MaxSensors)
+		{
+			String<StringLength20> sensorTypeName;
+			if (parser.GetStringParam('Y', sensorTypeName.GetRef()))
+			{
+				WriteLocker lock(sensorsLock);
+
+				DeleteSensor(sensorNum);
+
+				TemperatureSensor * const newSensor = TemperatureSensor::Create(sensorNum, CanInterface::GetCanAddress(), sensorTypeName.c_str(), reply);
+				if (newSensor == nullptr)
+				{
+					return GCodeResult::error;
+				}
+
+				const GCodeResult rslt = newSensor->Configure(parser, reply);
+				if (rslt == GCodeResult::ok || rslt == GCodeResult::warning)
+				{
+					InsertSensor(newSensor);
+				}
+				else
+				{
+					delete newSensor;
+				}
+				return rslt;
+			}
+
+			const auto sensor = FindSensor(sensorNum);
+			if (sensor.IsNull())
+			{
+				reply.printf("Sensor %u does not exist", sensorNum);
+				return GCodeResult::error;
+			}
+			return sensor->Configure(parser, reply);
+		}
+		else
+		{
+			reply.copy("Sensor number out of range");
+			return GCodeResult::error;
+		}
+	}
+
+	reply.copy("Missing sensor number parameter");
+	return GCodeResult::error;
+}
+
+#endif
+
 // End
diff --git a/src/Heating/Heat.h b/src/Heating/Heat.h
index aa680a49..3d86ed5e 100644
--- a/src/Heating/Heat.h
+++ b/src/Heating/Heat.h
@@ -142,9 +142,10 @@ public:
 #if SUPPORT_CAN_EXPANSION
 	void ProcessRemoteSensorsReport(CanAddress src, const CanMessageSensorTemperatures& msg) noexcept;
 	void ProcessRemoteHeatersReport(CanAddress src, const CanMessageHeatersStatus& msg) noexcept;
-# ifndef DUET3_ATE
+#endif
+
+#if SUPPORT_REMOTE_COMMANDS
 	GCodeResult EutProcessM308(const CanMessageGeneric& msg, const StringRef& reply) noexcept;
-# endif
 #endif
 
 	static ReadWriteLock sensorsLock;							// needs to be public so that the OMT in EndstopsManager can lock it
diff --git a/src/Heating/Sensors/SensorWithPort.cpp b/src/Heating/Sensors/SensorWithPort.cpp
index 004645e1..061bf901 100644
--- a/src/Heating/Sensors/SensorWithPort.cpp
+++ b/src/Heating/Sensors/SensorWithPort.cpp
@@ -8,6 +8,10 @@
 #include "SensorWithPort.h"
 #include "GCodes/GCodeBuffer/GCodeBuffer.h"
 
+#if SUPPORT_REMOTE_COMMANDS
+# include <CanMessageGenericParser.h>
+#endif
+
 SensorWithPort::SensorWithPort(unsigned int sensorNum, const char *type) noexcept
 	: TemperatureSensor(sensorNum, type)
 {
@@ -34,6 +38,27 @@ bool SensorWithPort::ConfigurePort(GCodeBuffer& gb, const StringRef& reply, PinA
 	return false;
 }
 
+#if SUPPORT_REMOTE_COMMANDS
+
+// Try to configure the port
+bool SensorWithPort::ConfigurePort(const CanMessageGenericParser& parser, const StringRef& reply, PinAccess access, bool& seen)
+{
+	String<StringLength20> portName;
+	if (parser.GetStringParam('P', portName.GetRef()))
+	{
+		seen = true;
+		return port.AssignPort(portName.c_str(), reply, PinUsedBy::sensor, access);
+	}
+	if (port.IsValid())
+	{
+		return true;
+	}
+	reply.copy("Missing port name parameter");
+	return false;
+}
+
+#endif
+
 // Copy the basic details to the reply buffer. This hides the version in the base class.
 void SensorWithPort::CopyBasicDetails(const StringRef& reply) const noexcept
 {
diff --git a/src/Heating/Sensors/SensorWithPort.h b/src/Heating/Sensors/SensorWithPort.h
index d627247c..4705b942 100644
--- a/src/Heating/Sensors/SensorWithPort.h
+++ b/src/Heating/Sensors/SensorWithPort.h
@@ -10,6 +10,8 @@
 
 #include "TemperatureSensor.h"
 
+class CanMessageGenericParser;
+
 class SensorWithPort : public TemperatureSensor
 {
 protected:
@@ -19,6 +21,11 @@ protected:
 	// Try to configure the port
 	bool ConfigurePort(GCodeBuffer& gb, const StringRef& reply, PinAccess access, bool& seen);
 
+#if SUPPORT_REMOTE_COMMANDS
+	// Try to configure the port
+	bool ConfigurePort(const CanMessageGenericParser& parser, const StringRef& reply, PinAccess access, bool& seen);
+#endif
+
 	// Copy the basic details to the reply buffer. This hides the version in the base class.
 	void CopyBasicDetails(const StringRef& reply) const noexcept;
 
diff --git a/src/Heating/Sensors/TemperatureSensor.cpp b/src/Heating/Sensors/TemperatureSensor.cpp
index 4785f9e2..aad12922 100644
--- a/src/Heating/Sensors/TemperatureSensor.cpp
+++ b/src/Heating/Sensors/TemperatureSensor.cpp
@@ -8,6 +8,10 @@
 #include "RemoteSensor.h"
 #include "GCodes/GCodeBuffer/GCodeBuffer.h"
 
+#if SUPPORT_REMOTE_COMMANDS
+# include <CanMessageGenericParser.h>
+#endif
+
 #if HAS_CPU_TEMP_SENSOR
 # include "CpuTemperatureSensor.h"
 #endif
@@ -99,6 +103,21 @@ GCodeResult TemperatureSensor::Configure(GCodeBuffer& gb, const StringRef& reply
 	return GCodeResult::ok;
 }
 
+#if SUPPORT_REMOTE_COMMANDS
+
+// Default implementation of Configure, for sensors that have no configurable parameters
+GCodeResult TemperatureSensor::Configure(const CanMessageGenericParser& parser, const StringRef& reply)
+{
+	if (!parser.HasParameter('Y'))
+	{
+		// No parameters were provided, so report the current configuration
+		CopyBasicDetails(reply);
+	}
+	return GCodeResult::ok;
+}
+
+#endif
+
 void TemperatureSensor::CopyBasicDetails(const StringRef& reply) const noexcept
 {
 	reply.printf("Sensor %u", sensorNumber);
@@ -166,7 +185,7 @@ TemperatureSensor *TemperatureSensor::Create(unsigned int sensorNum, const char
 {
 	TemperatureSensor *ts;
 #if SUPPORT_CAN_EXPANSION
-	if (boardAddress != 0)
+	if (boardAddress != CanInterface::GetCanAddress())
 	{
 		ts = new RemoteSensor(sensorNum, boardAddress);
 	}
diff --git a/src/Heating/Sensors/TemperatureSensor.h b/src/Heating/Sensors/TemperatureSensor.h
index abc51f76..f8d71ebc 100644
--- a/src/Heating/Sensors/TemperatureSensor.h
+++ b/src/Heating/Sensors/TemperatureSensor.h
@@ -8,6 +8,7 @@
 #include <ObjectModel/ObjectModel.h>
 
 class GCodeBuffer;
+class CanMessageGenericParser;
 struct CanSensorReport;
 
 class TemperatureSensor INHERIT_OBJECT_MODEL
@@ -34,6 +35,13 @@ public:
 	// if we find no relevant parameters, report the current parameters to 'reply' and return 'false'.
 	virtual GCodeResult Configure(GCodeBuffer& gb, const StringRef& reply, bool& changed);
 
+#if SUPPORT_REMOTE_COMMANDS
+	// Configure the sensor from M308 parameters.
+	// If we find any parameters, process them and return true. If an error occurs while processing them, return error and write an error message to 'reply.
+	// If we find no relevant parameters, report the current parameters to 'reply' and return ok.
+	virtual GCodeResult Configure(const CanMessageGenericParser& parser, const StringRef& reply);
+#endif
+
 #if SUPPORT_OBJECT_MODEL
 	// Report the sensor type in the form corresponding to the Y parameter of M308.
 	virtual const char *GetShortSensorType() const noexcept = 0;
diff --git a/src/Heating/Sensors/Thermistor.cpp b/src/Heating/Sensors/Thermistor.cpp
index 873e4325..43870c1f 100644
--- a/src/Heating/Sensors/Thermistor.cpp
+++ b/src/Heating/Sensors/Thermistor.cpp
@@ -16,6 +16,10 @@
 # include <Hardware/NonVolatileMemory.h>
 #endif
 
+#if SUPPORT_REMOTE_COMMANDS
+# include <CanMessageGenericParser.h>
+#endif
+
 #if SAME5x		// if using CoreN2G
 # include <AnalogIn.h>
 using AnalogIn::AdcBits;
@@ -58,6 +62,135 @@ int32_t Thermistor::GetRawReading(bool& valid) const noexcept
 	return (uint32_t)port.ReadAnalog() << AdcOversampleBits;
 }
 
+// Configure the H parameter returning true if successful, false if error
+bool Thermistor::ConfigureHParam(int hVal, const StringRef& reply) noexcept
+{
+	if (hVal == 999)
+	{
+#if HAS_VREF_MONITOR
+		bool valid;
+		const int32_t val = GetRawReading(valid);
+		if (valid)
+		{
+			int32_t vrefReading = reprap.GetPlatform().GetAdcFilter(VrefFilterIndex).GetSum();
+# ifdef DUET3
+			// Duet 3 MB6HC board revisions 0.6 and 1.0 have the series resistor connected to VrefP, not VrefMon, so extrapolate the VrefMon reading to estimate VrefP.
+			// Version 1.01 and later boards have the series resistors connected to VrefMon.
+			if (reprap.GetPlatform().GetBoardType() == BoardType::Duet3_v06_100)
+			{
+				const int32_t vssaReading = reprap.GetPlatform().GetAdcFilter(VssaFilterIndex).GetSum();
+				vrefReading = vssaReading + lrintf((vrefReading - vssaReading) * (4715.0/4700.0));
+			}
+# endif
+			const int32_t computedCorrection =
+							(val - (int32_t)(vrefReading/(ThermistorAveragingFilter::NumAveraged() >> AdcOversampleBits)))
+								/(1 << (AdcBits + AdcOversampleBits - 13));
+			if (computedCorrection >= -127 && computedCorrection <= 127)
+			{
+				adcHighOffset = (int8_t)computedCorrection;
+				reply.copy("Measured H correction for port \"");
+				port.AppendPinName(reply);
+				reply.catf("\" is %d", adcHighOffset);
+
+				// Store the value in NVM
+				if (!reprap.GetGCodes().IsRunningConfigFile())
+				{
+					NonVolatileMemory mem;
+					mem.SetThermistorHighCalibration(adcFilterChannel, adcHighOffset);
+					mem.EnsureWritten();
+				}
+			}
+			else
+			{
+				reply.copy("Computed correction is not valid. Check that you have disconnected the thermistor.");
+				return false;
+			}
+		}
+		else
+		{
+			reply.copy("Temperature reading is not valid");
+			return false;
+		}
+#else
+		reply.copy("Thermistor input auto calibration is not supported by this hardware");
+		return false;
+#endif
+	}
+	else
+	{
+		adcHighOffset = (int8_t)constrain<int>(hVal, std::numeric_limits<int8_t>::min(), std::numeric_limits<int8_t>::max());
+	}
+	return true;
+}
+
+// Configure the L parameter returning true if successful, false if error
+bool Thermistor::ConfigureLParam(int lVal, const StringRef& reply) noexcept
+{
+	if (lVal == 999)
+	{
+#if HAS_VREF_MONITOR
+		bool valid;
+		const int32_t val = GetRawReading(valid);
+		if (valid)
+		{
+			const int32_t computedCorrection =
+							(val - (int32_t)(reprap.GetPlatform().GetAdcFilter(VssaFilterIndex).GetSum()/(ThermistorAveragingFilter::NumAveraged() >> AdcOversampleBits)))
+								/(1 << (AdcBits + AdcOversampleBits - 13));
+			if (computedCorrection >= -127 && computedCorrection <= 127)
+			{
+				adcLowOffset = (int8_t)computedCorrection;
+				reply.copy("Measured L correction for port \"");
+				port.AppendPinName(reply);
+				reply.catf("\" is %d", adcLowOffset);
+
+				// Store the value in NVM
+				if (!reprap.GetGCodes().IsRunningConfigFile())
+				{
+					NonVolatileMemory mem;
+					mem.SetThermistorLowCalibration(adcFilterChannel, adcLowOffset);
+					mem.EnsureWritten();
+				}
+			}
+			else
+			{
+				reply.copy("Computed correction is not valid. Check that you have placed a jumper across the thermistor input.");
+				return false;
+			}
+		}
+		else
+		{
+			reply.copy("Temperature reading is not valid");
+			return false;
+		}
+#else
+		reply.copy("Thermistor input auto calibration is not supported by this hardware");
+		return false;
+#endif
+	}
+	else
+	{
+		adcLowOffset = (int8_t)constrain<int>(lVal, std::numeric_limits<int8_t>::min(), std::numeric_limits<int8_t>::max());
+	}
+	return true;
+}
+
+// Initialise parameters after changing the port
+void Thermistor::InitPort() noexcept
+{
+	adcLowOffset = adcHighOffset = 0;
+	adcFilterChannel = reprap.GetPlatform().GetAveragingFilterIndex(port);
+	if (adcFilterChannel >= 0)
+	{
+		reprap.GetPlatform().GetAdcFilter(adcFilterChannel).Init((1u << AdcBits) - 1);
+#if HAS_VREF_MONITOR
+		// Default the H and L parameters to the values from nonvolatile memory
+		NonVolatileMemory mem;
+		adcLowOffset = mem.GetThermistorLowCalibration(adcFilterChannel);
+		adcHighOffset = mem.GetThermistorHighCalibration(adcFilterChannel);
+#endif
+	}
+}
+
 // Configure the temperature sensor
 GCodeResult Thermistor::Configure(GCodeBuffer& gb, const StringRef& reply, bool& changed)
 {
@@ -68,19 +201,7 @@ GCodeResult Thermistor::Configure(GCodeBuffer& gb, const StringRef& reply, bool&
 
 	if (changed)
 	{
-		// We changed the port, so clear the ADC corrections and set up the ADC filter if there is one
-		adcLowOffset = adcHighOffset = 0;
-		adcFilterChannel = reprap.GetPlatform().GetAveragingFilterIndex(port);
-		if (adcFilterChannel >= 0)
-		{
-			reprap.GetPlatform().GetAdcFilter(adcFilterChannel).Init((1u << AdcBits) - 1);
-#if HAS_VREF_MONITOR
-			// Default the H and L parameters to the values from nonvolatile memory
-			NonVolatileMemory mem;
-			adcLowOffset = mem.GetThermistorLowCalibration(adcFilterChannel);
-			adcHighOffset = mem.GetThermistorHighCalibration(adcFilterChannel);
-#endif
-		}
+		InitPort();							// we changed the port, so clear the ADC corrections and set up the ADC filter if there is one
 	}
 
 	gb.TryGetFValue('R', seriesR, changed);
@@ -103,112 +224,18 @@ GCodeResult Thermistor::Configure(GCodeBuffer& gb, const StringRef& reply, bool&
 
 	if (gb.Seen('L'))
 	{
-		const int lVal = gb.GetIValue();
-		if (lVal == 999)
+		if (!ConfigureLParam(gb.GetIValue(), reply))
 		{
-#if HAS_VREF_MONITOR
-			bool valid;
-			const int32_t val = GetRawReading(valid);
-			if (valid)
-			{
-				const int32_t computedCorrection =
-								(val - (int32_t)(reprap.GetPlatform().GetAdcFilter(VssaFilterIndex).GetSum()/(ThermistorAveragingFilter::NumAveraged() >> AdcOversampleBits)))
-									/(1 << (AdcBits + AdcOversampleBits - 13));
-				if (computedCorrection >= -127 && computedCorrection <= 127)
-				{
-					adcLowOffset = (int8_t)computedCorrection;
-					reply.copy("Measured L correction for port \"");
-					port.AppendPinName(reply);
-					reply.catf("\" is %d", adcLowOffset);
-
-					// Store the value in NVM
-					if (!reprap.GetGCodes().IsRunningConfigFile())
-					{
-						NonVolatileMemory mem;
-						mem.SetThermistorLowCalibration(adcFilterChannel, adcLowOffset);
-						mem.EnsureWritten();
-					}
-				}
-				else
-				{
-					reply.copy("Computed correction is not valid. Check that you have placed a jumper across the thermistor input.");
-					return GCodeResult::error;
-				}
-			}
-			else
-			{
-				reply.copy("Temperature reading is not valid");
-				return GCodeResult::error;
-			}
-#else
-			reply.copy("Thermistor input auto calibration is not supported by this hardware");
 			return GCodeResult::error;
-#endif
-		}
-		else
-		{
-			adcLowOffset = (int8_t)constrain<int>(lVal, std::numeric_limits<int8_t>::min(), std::numeric_limits<int8_t>::max());
 		}
 		changed = true;
 	}
 
 	if (gb.Seen('H'))
 	{
-		const int hVal = gb.GetIValue();
-		if (hVal == 999)
+		if (!ConfigureHParam(gb.GetIValue(), reply))
 		{
-#if HAS_VREF_MONITOR
-			bool valid;
-			const int32_t val = GetRawReading(valid);
-			if (valid)
-			{
-				int32_t vrefReading = reprap.GetPlatform().GetAdcFilter(VrefFilterIndex).GetSum();
-#ifdef DUET3
-				// Duet 3 MB6HC board revisions 0.6 and 1.0 have the series resistor connected to VrefP, not VrefMon, so extrapolate the VrefMon reading to estimate VrefP.
-				// Version 1.01 and later boards have the series resistors connected to VrefMon.
-				if (reprap.GetPlatform().GetBoardType() == BoardType::Duet3_v06_100)
-				{
-					const int32_t vssaReading = reprap.GetPlatform().GetAdcFilter(VssaFilterIndex).GetSum();
-					vrefReading = vssaReading + lrintf((vrefReading - vssaReading) * (4715.0/4700.0));
-				}
-#endif
-				const int32_t computedCorrection =
-								(val - (int32_t)(vrefReading/(ThermistorAveragingFilter::NumAveraged() >> AdcOversampleBits)))
-									/(1 << (AdcBits + AdcOversampleBits - 13));
-				if (computedCorrection >= -127 && computedCorrection <= 127)
-				{
-					adcHighOffset = (int8_t)computedCorrection;
-					reply.copy("Measured H correction for port \"");
-					port.AppendPinName(reply);
-					reply.catf("\" is %d", adcHighOffset);
-
-					// Store the value in NVM
-					if (!reprap.GetGCodes().IsRunningConfigFile())
-					{
-						NonVolatileMemory mem;
-						mem.SetThermistorHighCalibration(adcFilterChannel, adcHighOffset);
-						mem.EnsureWritten();
-					}
-				}
-				else
-				{
-					reply.copy("Computed correction is not valid. Check that you have disconnected the thermistor.");
-					return GCodeResult::error;
-				}
-			}
-			else
-			{
-				reply.copy("Temperature reading is not valid");
-				return GCodeResult::error;
-			}
-#else
-			reply.copy("Thermistor input auto calibration is not supported by this hardware");
 			return GCodeResult::error;
-#endif
-		}
-		else
-		{
-			adcHighOffset = (int8_t)constrain<int>(hVal, std::numeric_limits<int8_t>::min(), std::numeric_limits<int8_t>::max());
 		}
 		changed = true;
 	}
@@ -245,6 +272,78 @@ GCodeResult Thermistor::Configure(GCodeBuffer& gb, const StringRef& reply, bool&
 	return GCodeResult::ok;
 }
 
+#if SUPPORT_REMOTE_COMMANDS
+
+// Configure the temperature sensor
+GCodeResult Thermistor::Configure(const CanMessageGenericParser& parser, const StringRef& reply)
+{
+	bool changed = false;
+	if (!ConfigurePort(parser, reply, PinAccess::readAnalog, changed))
+	{
+		return GCodeResult::error;
+	}
+
+	if (changed)
+	{
+		InitPort();							// we changed the port, so clear the ADC corrections and set up the ADC filter if there is one
+	}
+
+	changed = parser.GetFloatParam('R', seriesR) || changed;
+	if (!isPT1000)
+	{
+		if (parser.GetFloatParam('B', beta))
+		{
+			shC = 0.0;						// if user changes B and doesn't define C, assume C=0
+			changed = true;
+		}
+		changed = parser.GetFloatParam('C', shC) || changed;
+		changed = parser.GetFloatParam('T', r25) || changed;
+		if (changed)
+		{
+			CalcDerivedParameters();
+		}
+	}
+
+	int16_t lVal;
+	if (parser.GetIntParam('L', lVal))
+	{
+		if (!ConfigureLParam(lVal, reply))
+		{
+			return GCodeResult::error;
+		}
+		changed = true;
+	}
+
+	int16_t hVal;
+	if (parser.GetIntParam('H', hVal))
+	{
+		if (!ConfigureHParam(hVal, reply))
+		{
+			return GCodeResult::error;
+		}
+		changed = true;
+	}
+
+	if (!changed)
+	{
+		CopyBasicDetails(reply);
+		if (isPT1000)
+		{
+			// For a PT1000 sensor, only the series resistor is configurable
+			reply.catf(", R:%.1f", (double)seriesR);
+		}
+		else
+		{
+			reply.catf(", T:%.1f B:%.1f C:%.2e R:%.1f", (double)r25, (double)beta, (double)shC, (double)seriesR);
+		}
+		reply.catf(" L:%d H:%d", adcLowOffset, adcHighOffset);
+	}
+
+	return GCodeResult::ok;
+}
+
+#endif
+
 // Get the temperature
 void Thermistor::Poll() noexcept
 {
diff --git a/src/Heating/Sensors/Thermistor.h b/src/Heating/Sensors/Thermistor.h
index 9f7249ff..ce1b0e01 100644
--- a/src/Heating/Sensors/Thermistor.h
+++ b/src/Heating/Sensors/Thermistor.h
@@ -22,7 +22,13 @@ class Thermistor : public SensorWithPort
 {
 public:
 	Thermistor(unsigned int sensorNum, bool p_isPT1000) noexcept;					// create an instance with default values
-	GCodeResult Configure(GCodeBuffer& gb, const StringRef& reply, bool& changed) override THROWS(GCodeException); // configure the sensor from M305 parameters
+
+	GCodeResult Configure(GCodeBuffer& gb, const StringRef& reply, bool& changed) override THROWS(GCodeException); // configure the sensor from M308 parameters
+
+#if SUPPORT_REMOTE_COMMANDS
+	GCodeResult Configure(const CanMessageGenericParser& parser, const StringRef& reply) override; // configure the sensor from M308 parameters
+#endif
+
 	void Poll() noexcept override;
 	const char *GetShortSensorType() const noexcept override { return (isPT1000) ? TypeNamePT1000 : TypeNameThermistor; }
 
@@ -32,6 +38,9 @@ public:
 private:
 	void CalcDerivedParameters() noexcept;											// calculate shA and shB
 	int32_t GetRawReading(bool& valid) const noexcept;								// get the ADC reading
+	bool ConfigureHParam(int hVal, const StringRef& reply) noexcept;				// configure the H parameter returning true if successful, false if error
+	bool ConfigureLParam(int lVal, const StringRef& reply) noexcept;				// configure the L parameter returning true if successful, false if error
+	void InitPort() noexcept;
 
 	// The following are configurable parameters
 	float r25, beta, shC, seriesR;													// parameters declared in the M305 command