Improve the diagnostic for temperature values and remove calib14 debug lines

[MSECode]

Is your feature request related to a problem?

As highlighted in here: robotology/icub-tech-support#1987 (comment), the diagnostic message for the temperature errors need to be revised as follows:

• timestamp should be formatted to a readable relative time or, if we wanna keep it absolute, with readable time and data and not as just a number in milliseconds.
• we can think to move the errors to warnings since they are non blocking and they do not move the joint in fault.

Moreover, we need to remove the following debug lines from the calibration 14 logs, since we do not need them anymore:

icub-main/src/libraries/icubmod/embObjMotionControl/embObjMotionControl.cpp

Lines 2285 to 2290 in 22c06ce

	yDebug() << "***** calib.params.type14.pwmlimit = " <<calib.params.type14.pwmlimit;
	yDebug() << "***** calib.params.type14.final_pos = " <<calib.params.type14.final_pos;
	yDebug() << "***** calib.params.type14.invertdirection = " <<calib.params.type14.invertdirection;
	yDebug() << "***** calib.params.type14.rotation = " <<calib.params.type14.rotation;
	yDebug() << "***** calib.params.type14.offset = " <<calib.params.type14.offset;
	yDebug() << "***** calib.params.type14.calibrationZero = " <<calib.params.type14.calibrationZero;

The solution you would like to have available

Improve diagnostic by:

• formatting well the timestamp to a readable string for the temperature related issue
• add the converted temperature in celsius, so that the user knows the actual value of the raw temperature sent in the logs
• remove the unneeded logs from the calibration 14

Alternatives you have considered

No response

Additional context

No response

[MSECode]

Hi @valegagge,
ragarding timestamp and debug string all ok.
Considering instead the issue with the conversion of the raw temperature to Celsius at this section of the parser:

icub-main/src/libraries/icubmod/embObjLib/diagnosticInfoParsers.cpp

Lines 704 to 715 in 22c06ce

	case eoerror_value_MC_motor_overheating:
	{
	uint16_t joint_num = m_dnginfo.param16;
	int16_t temp_raw_value = m_dnginfo.param64 & 0xffff;
	m_entityNameProvider.getAxisName(joint_num, m_dnginfo.baseInfo.axisName);
	snprintf(str, sizeof(str), " %s (Joint=%s (NIB=%d), Raw_temperature_value=%d)",
	m_dnginfo.baseMessage.c_str(), m_dnginfo.baseInfo.axisName.c_str(), joint_num, temp_raw_value
	);
	m_dnginfo.baseInfo.finalMessage.append(str);
	} break;

,
we would make use of the methods of the interface: yarp::dev::eomc::ITemperatureSensor, which have already been overridden in here:

icub-main/src/libraries/icubmod/embObjMotionControl/eomcParser.h

Lines 79 to 233 in 22c06ce

	class TemperatureSensorPT100 : public ITemperatureSensor
	{
	private:
	// resistors of the voltage divider bridge
	int _r_1;
	int _r_2;
	int _r_3;
	double _ptc_offset; // offset of the temperature sensor line
	double _ptc_gradient; // slope/gradient of the temperature sensor line
	double _pga_gain; // ADC gain set for the tdb (temperature detection board)
	int _vcc; // vcc that enters the voltage divider bridge
	double _resolution_pga; // resolution of the internal pga of the tdb
	double _resolution_tdb; // resolution used for the raw value for the output of the tdb
	double _half_bridge_resistor_coeff;
	double _first_res_coeff;
	double _second_res_coeff;
	public:
	TemperatureSensorPT100()
	{
	_r_1 = 4700;
	_r_2 = 4700;
	_r_3 = 100;
	_ptc_offset = 100.0;
	_ptc_gradient = 0.3851;
	_pga_gain = 2;
	_vcc = 5;
	_resolution_pga = 2.048;
	_resolution_tdb = 32767;
	// define here and calculate when the class object is built to speed up calculations
	// since the value does not change in the sensor type class object
	_half_bridge_resistor_coeff = (double)_r_3 / (double)(_r_2 + _r_3);
	_first_res_coeff = _r_1*_r_2 + _r_1*_r_3 + _ptc_offset*_r_2 + _ptc_offset*_r_3;
	_second_res_coeff = _r_3*_r_1 - _r_2*_ptc_offset;
	}
	TemperatureSensorPT100(const TemperatureSensorPT100& other) = default; // const copy constructor
	TemperatureSensorPT100& operator=(const TemperatureSensorPT100& other) = default; // const copy assignment operator
	TemperatureSensorPT100(TemperatureSensorPT100&& other) = default; // move constructor
	TemperatureSensorPT100& operator=(TemperatureSensorPT100&& other) = default; // move assignment operator
	~TemperatureSensorPT100() = default; // destructor
	virtual double convertTempCelsiusToRaw(const double temperature) override
	{
	double res = 0;
	double tmp = (( (_ptc_offset + _ptc_gradient * temperature) / ((double)_r_1 + (_ptc_offset + _ptc_gradient * temperature))) - _half_bridge_resistor_coeff) * (double)_vcc;
	res = (_resolution_tdb + 1) * ((_pga_gain * tmp) / _resolution_pga);
	yDebug("Converted temperature limit to raw value:%f", res);
	return res;
	}
	virtual double convertRawToTempCelsius(const double temperature) override
	{
	double res = 0;
	double tmp = temperature * ((_resolution_pga) / (_pga_gain * _vcc * (_resolution_tdb + 1)));
	double den = _ptc_gradient * (_r_2 - _r_2*tmp - _r_3*tmp);
	res = (tmp * (_first_res_coeff) / den) + ((_second_res_coeff) / den);
	return res;
	}
	virtual motor_temperatureSensorTypeEnum_t getType() override
	{return motor_temperature_sensor_pt100;}
	};
	class TemperatureSensorPT1000 : public ITemperatureSensor
	{
	private:
	// resistors of the voltage divider bridge
	int _r_1;
	int _r_2;
	int _r_3;
	double _ptc_offset; // offset of the temperature sensor line
	double _ptc_gradient; // slope/gradient of the temperature sensor line
	double _pga_gain; // ADC gain set for the tdb (temperature detection board)
	int _vcc; // vcc that enters the voltage divider bridge
	double _resolution_pga; // resolution of the internal pga of the tdb
	double _resolution_tdb; // resolution used for the raw value for the output of the tdb
	// define here and calculate when the class object is built to speed up calculations
	// since the value does not change in the sensor type class object
	double _half_bridge_resistor_coeff;
	double _first_res_coeff;
	double _second_res_coeff;
	public:
	TemperatureSensorPT1000()
	{
	_r_1 = 4700;
	_r_2 = 4700;
	_r_3 = 1000;
	_ptc_offset = 1000;
	_ptc_gradient = 3.851;
	_pga_gain = 2;
	_vcc = 5;
	_resolution_pga = 2.048;
	_resolution_tdb = 32767;
	_half_bridge_resistor_coeff = (double)_r_3 / (double)(_r_2 + _r_3);
	_first_res_coeff = _r_1*_r_2 + _r_1*_r_3 + _ptc_offset*_r_2 + _ptc_offset*_r_3;
	_second_res_coeff = _r_3*_r_1 - _r_2*_ptc_offset;
	}
	TemperatureSensorPT1000(const TemperatureSensorPT1000& other) = default; // const copy constructor
	TemperatureSensorPT1000& operator=(const TemperatureSensorPT1000& other) = default; // const copy assignment operator
	TemperatureSensorPT1000(TemperatureSensorPT1000&& other) = default; // move constructor
	TemperatureSensorPT1000& operator=(TemperatureSensorPT1000&& other) = default; // move assignment operator
	~TemperatureSensorPT1000() = default; // destructor
	virtual double convertTempCelsiusToRaw(const double temperature) override
	{
	double res = 0;
	double tmp = (( (_ptc_offset + _ptc_gradient * temperature) / ((double)_r_1 + (_ptc_offset + _ptc_gradient * temperature))) - _half_bridge_resistor_coeff) * (double)_vcc;
	res = (_resolution_tdb + 1) * ((_pga_gain * tmp) / _resolution_pga);
	yDebug("Converted temperature limit to raw value:%f", res);
	return res;
	}
	virtual double convertRawToTempCelsius(const double temperature) override
	{
	double res = 0;
	double tmp = temperature * ((_resolution_pga) / (_pga_gain * _vcc * (_resolution_tdb + 1)));
	double den = _ptc_gradient * (_r_2 - _r_2*tmp - _r_3*tmp);
	res = (tmp * (_first_res_coeff) / den) + ((_second_res_coeff) / den);
	return res;
	}
	virtual motor_temperatureSensorTypeEnum_t getType() override
	{return motor_temperature_sensor_pt1000;}
	};

, depending on the type of sensor.
Thus, to exploit them, I was thinking to add in the constructor of Diagnostic::LowLevel::MotionControlParser a unique_ptr to each of the override of yarp::dev::eomc::ITemperatureSensor thus to use the method: virtual double convertRawToTempCelsius(const double temperature)

Does it make sense? Can this lead to problem of circular dependencies considering that the diagnostic is part of embObjLib and eomcParser is part of embObjMotionControl? Is there a better manner to do this without the need of re-overriding the base interface?
We can discuss about that the next days.

[valegagge]

@MSECode , thanks. It is clear. We are going to talk as soon as possible!

[MSECode]

After discussion we proceed in fixed the following bugs:

• timestamps error for the temperatures errors and set them as warnings. Here the error: ergoCub 1.3 S/N:002 – Weird smell coming from the robot and errors on the right leg icub-tech-support#1987 (comment)
• removal of debug lines for calibration type 14

Regarding the conversion of the temperatures from raw to Celsius on the errors will be addressed later since it cannot be currently done without dealing with circular dependencies on icub-main.

The updates have been done at this branch: https://github.com/MSECode/icub-main/tree/feature/updateDiagnosticTempAndCalib14