arendst / Tasmota

Alternative firmware for ESP8266 and ESP32 based devices with easy configuration using webUI, OTA updates, automation using timers or rules, expandability and entirely local control over MQTT, HTTP, Serial or KNX. Full documentation at
https://tasmota.github.io/docs
GNU General Public License v3.0
21.8k stars 4.73k forks source link

add options for INA3221 driver #21285

Closed fb-pilot closed 3 months ago

fb-pilot commented 3 months ago

Description:

more defines for sensor100 INA3221 driver and extend documentation Related issue (if applicable): fixes #21262

Checklist:

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

fb-pilot commented 3 months ago

here we go - as discussed with the driver

barbudor commented 3 months ago

1) there is a typo in NA3221_SUPPLY_SIDE 2) INA3221_SUPPLY_SIDE does nothing, not implemeted in the driver 3) Although I understand what you wanted to do and would see no issue with implementing that work mode, DO NOT CHANGE THE DEFAULT BEHAVIOR as this is a breaking change for other users. Do it in a way that it can be changed in user_config_override.h only for those interrested

fb-pilot commented 3 months ago

changed ... What do you mean with "not implemeted in the driver" ? Is something messed up ?

fb-pilot commented 3 months ago

I think we also should change the title .... but have no idea how to !?

barbudor commented 3 months ago

As I commented in your PR, the PR title was not referring INA3221 so I hadn't had a change to review it before it is merged and your changed were not visible on my computer

I'll now review your PR and still comment

barbudor commented 3 months ago

Owner of the PR should see an edit button on the right of a PR title

fb-pilot commented 3 months ago

Ahh - who can read is clearly in advantage ... corrected my mistakes :-))

barbudor commented 3 months ago

You need to learn how to use git and branches

fb-pilot commented 3 months ago

You need to learn how to use git and branches

I see .... I am completely lost now :-((