INA3221 fix display multiple devices and add more options

[fb-pilot]

fix display multiple devices and add more options

Description:

optimised code to save about 500 byte and used it for additional options
Related issue (if applicable): fixes #

Checklist:

• The pull request is done against the latest development branch
• Only relevant files were touched
• Only one feature/fix was added per PR and the code change compiles without warnings
• The code change is tested and works with Tasmota core ESP8266 V.2.7.6
• The code change is tested and works with Tasmota core ESP32 V.3.0.0
• I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

[s-hadinger]

Thanks, looks nice. One naive question (I don't own the device), why is INA3221_SUPPLY_SIDE a compile time option? Would it be more flexible to have it as a runtime option? In which case is it needed?

[fb-pilot]

thanks for your commendation :-) .... I added it ( INA3221_SUPPLY_SIDE) as new feature so the driver is compatible to te former version - once you defined it you can choose in the command "sensor100" (by putting positive or negativ values for the shuntresistor) per channel whitch side of the shunt the busvoltage is measured / calculated.

[s-hadinger]

So shouldn't it be enabled by default (which is not the case here)? What's the harm if you always enable it?

[fb-pilot]

Your second Question .... if you have a powersupply and a load - the shuntvoltage is the difference between the two voltages. With this feature enabled you can choose which voltage shoud be considered independent of the current (= voltagedrop on the shunt) flowing. That also concerns the determined power and energy .... it is the power supplied by the powersupply or the power consumed by the load.

[fb-pilot]

ok I see no harm - exept al little mor code. We for shure can set the define by default. But the behavior of the driver is different as I also put the activation of the busvoltage in this option. That means the driver always or never scans and shows the voltage - independent of the definition of the according shuntresistor which then influences only the scan of the current

[s-hadinger]

Ok, I'll let you to your judgment if it's better to enable by default. Think of legacy but also future users. Please tell me and I merge right after

[fb-pilot]

I have no feeling if it produces complaints if we provide a driver with incompatibel behavior to the existing ? If we set it by default one can't reset it in the user_config_override.h .... so I can set it by default and see what happens .... what do you think ?

[fb-pilot]

another idea is to put the #define INA3221_SUPPLY_SIDE in the user_config_override.h - and extend the documentation at least of sensor100 - where else ?
What's your opinion ?

[s-hadinger]

another idea is to put the #define INA3221_SUPPLY_SIDE in the user_config_override.h - and extend the documentation at least of sensor100 - where else ? What's your opinion ?

Yes, please do so

[fb-pilot]

Ok .... so you can merge the driver (as it is - we then have legacy behavior) and I'll extend the dokumentation. I did not do that till now - where should I start to read how to ?

[barbudor]

I'm very un happy that the name of this PR do not included a reference to the INA3221 driver which prevented me to have a deeper look before it is merged

I'll will still have a look

[Jason2866]

Driver fails to compile when linking.