letscontrolit / ESPEasy

Easy MultiSensor device based on ESP8266/ESP32
http://www.espeasy.com
Other
3.3k stars 2.22k forks source link

P027 - INA219 #321

Closed Grovkillen closed 7 years ago

Grovkillen commented 7 years ago

The plugin does not have any controller settings. Thus the values won't get published. See screenshot from latest dev10.

PS. Also, in general I think "Enable" should be the last option in the list since it is where I choose if I want to start the device or not, this happens right before I hit submit. PICTURE

Grovkillen commented 7 years ago

Double post of #311 ?

Grovkillen commented 7 years ago

Please see this discussion for more information: https://www.letscontrolit.com/forum/viewtopic.php?f=6&t=3175

We need to make the plugin more clear of what the different values mean. (Just internal documentation in the webgui)

ghost commented 7 years ago

The plugin was designed to send a single value per task as Domoticz has no custom triple value devices. Is it verified to still work with Domoticz?

Grovkillen commented 7 years ago

It works if used rules, but no... only one IDX per value is accepted by Domoticz.

psy0rz commented 7 years ago

woops... @krikk ?

krikk commented 7 years ago

if a sensor has 3 values, we should send 3 values... we should not cripple the sender, just because the receiver does not accept it. and when i understand @Grovkillen right you can work around it with rules...

this should be fixed in Domoticz not in esp easy...

krikk commented 7 years ago

and perhaps we should document the needed workaround solution if possible... @Grovkillen can you do that? ...because i do not have knowledge about domoticz...

ghost commented 7 years ago

Easy to state that it should be fixed in Domoticz, but it's not likely going to happen. Also note that it may well be that 90% of our community uses Domoticz. To maintain backwards compatibility it would be better to have a selection box on what value(s) to report. (current,voltage,power,all) Look at the P003 plugin to see how it was done there.

Remember that ESP Easy was build to make things easy (for the end user)...

krikk commented 7 years ago

i would prefer a more generic solution, like the ability to select which values are sent to the controller via a checkbox int the values list...

ghost commented 7 years ago

That may even be better, although harder to implement as we currently have this SENSORTYPE mechanisme. This was build when we only supported Domoticz and Domoticz uses an update api with different syntax for different sensor types.

Being able to select individual values to report, you'd have to figure out in code how to translate this to the proper SENSOR_TYPE.

Would it be wise to build this as a generic device setting on all devices? I think it affects all device and controller plugin code, as well as some core code...

Grovkillen commented 7 years ago

So what do we say, should I document it in the rules section? It might be good to know even if it should not be an official workaround?

ghost commented 7 years ago

I'd prefer to restore the original selection box and just add the possibility to report "All". Looks the most simple and convenient solution, at least for short term (so all devs can focus on getting a stable release)

General recommendation: If changes break existing functionality, try to make the change an optional feature using a dropdown config or tick box. We have always made quite some effort to remain backwards compatible with older releases so we do not loose existing happy campers...

We should try to avoid using rules to workaround things that we change, as it makes things complicated for regular users.

Grovkillen commented 7 years ago

I made a tutorial anyways since it might be good to be able to create custom publishing to Domoticz: https://www.letscontrolit.com/wiki/index.php/Tutorial_Rules#Custom_reports_to_Domoticz_with_own_IDX

psy0rz commented 7 years ago

what if we make the domoticz plugin just send the first value of an triplevalue to domoticz?

i agree with martinus that wd should stay backwards compatible for now.

Grovkillen commented 7 years ago

That's the winner suggestion in my opinion. But will that mean that only Volt is possible for the INA219 plugin or is it possible to tell which one that is the first value?

krikk commented 7 years ago

@Grovkillen nailed the problem, thats the reason i implemeted the selection box see... see pull request...

hope this works