Tasmota: Development best practice advice

[claybar]

I would like some clarity on how to tackle potentially changing existing functionality of the code. Below is a bit of a story of changes I have made to the MPU6050 code to suit my needs originally posted on Discord. This got me thinking of how best to form this into a PR.

I am in the process of modifying the driver code for the MPU6050 to provide yaw/pitch/roll (YPR) measurements. I would like a bit of advice on how best to structure the final output of this. Currently the sensor operates in two modes based on #defines. The basic mode output the raw accel/gyro values as 6 values, both to the webserver and JSON/MQTT. If a #define is set to enable the DPU the output now changes to accelerations with gravity removed, and Euler angles in place of the gyro values. Note, the JSON variable name is not changed in this case.

I want to add a third set of numbers, the YPR. I could follow the pattern established and add another #define that replaces the Euler angles with YPR, but this seems a bit weird. Alternatively this #define could add additional fields to the output to contain the YPR values. Now the sensor will output 10 values, the 6 above, 3 new YPR, and the temperature. Is this too many?

While I am there I could also change the current behavior so the real accel (grav removed) and Euler angles get their own fields when enabled, but this would potentially break existing use cases.

[arendst]

Extending the JSON message with additional fields is no problem. It provides backward compatibility by adding new fields.

The only issue is if the total sensor status message (this is all sensor data in one string) becomes too long. Over MIN_MESSZ (=893) characters will provide a problem as it will be truncated and make the JSON message no-conformative.

So as long users are not using too many sensors you should be fine.

Changing the meaniong of a field is a no-go as processors do not know that the meaning has changed. So you will have to provide new field names

In the case of MPU6050 why not choose a message like

"MPU6050":{"Temperature":20.2, ... , "DPU":[Yvalue,Pvalue,Rvalue]}

It would have been better if was like from the beginning:

"MPU6050":{"Temperature":20.2,"Acceleration":[Xvalue,Yvalue,Zvalue],"Gyro":[Xvalue,Yvalue,Zvalue],"DPU":[Yvalue,Pvalue,Rvalue]}

[lalo-uy]

Just keep in mind that the Temp in the MPU6050 is the inner CPU temp, not very useful