letscontrolit / ESPEasy

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

Why does Generic http Publish multiple times at once? #1470

Closed shin4299 closed 10 months ago

shin4299 commented 6 years ago

I set up controllers Protocol: Generic HTTP Controller Publish: {"%vname1%":%val1%,"%vname2%":%val2%,"%vname3%":%val3%} Send Interval: 30 seconds

As in the example above, if the values are 3, ESP will be publish 3 times at once. If the value is one, ESP will be publish one time at once.

Regardless of the number of values, ESP must be publish only once. Please fix it.

TD-er commented 6 years ago

Is that the literal string you use?

When selecting that controller, the suggested string is this: demo.php?name=%sysname%&task=%tskname%&valuename=%valname%&value=%value%

I am not sure how it should send this JSON formatted string you use.

It looks rather meant to be this way: https://github.com/letscontrolit/ESPEasy/blob/0fc3a28875e5cd51d901f3d038ba2729e9c593f7/src/_C008.ino#L42-L60

Since I changed this formatting of the userVar only a few weeks ago, I also looked into that commit to see what it was before and its functionality is not changed (only less is sent when measured value is invalid): https://github.com/letscontrolit/ESPEasy/commit/30acf569e7b2b4bbb00599c94ecb1c9c452cc5ad#diff-8e064063b0d897d2f8044f93dee67e42L50

Maybe you want to use the advanced version of this Generic HTTP controller? It looks like such a JSON formatting should be sent in a POST request and not in a GET request this controller does.

shin4299 commented 6 years ago

The number of publish is 3(if values is 3), regardless of whether it is a string in JSON format or not(suggested string).

And the advanced mode is the same. If the values are 3, ESP publishes 3 times at onece.

The bigger problem is that the advanced mode is very unstable.

If I change the Generic HTTP mode to the advanced mode, ESP will continue to reboot.

pkoster commented 4 years ago

I just ran into the same issue. The HTTP Generic controller will invoke a HTTP-GET per sensor variable rather than just once.

Just like above issue reporter, in my use case this is undesirable behavior. Actually, I can hardly think of a use case where this behavior is desirable. One HTTP-GET per sensor sampling interval seems to make sense.

For example, the Gases - CO2 MH-Z19 sensor has 3 variables which are sampled every 60 seconds. Lets assume we push only the first (PPM) respectively first and second (PPM + temp) variables to a PHP script with the HTTP Generic controller:

influx.php?db=test&q=co2,location%3Dmasterbedroom%20co2%3D%val1%
influx.php?db=test&q=co2,location%3Dmasterbedroom%20co2%3D%val1%,temp%3D%val2%`

In both cases this results in 3 identical HTTP-GET invocations.

The fix at first sight could be simple by taking the HTTPSend etc, out of the loop. (Probably some consideration is appropriate to the support of %valname% / %value% apposed to %vname1% / %val1% etc.)

TD-er commented 4 years ago

You can have a look at the HTTP Advanced controller, or create an URL yourself and call it from the rules. See the Command Reference for SendToHttp

pkoster commented 4 years ago

Actually, I ran into this in search for an alternative to the unstable HTTP Advanced controller (https://github.com/letscontrolit/ESPEasy/issues/2993).

Next to suggestions for alternative means to achieve the goal (which is appreciated), it would be useful to have a statement / clarification on the intended behavior of this plugin. The behavior (whether by design, legacy, or otherwise) does not seem to match expectations (at least for a casual user).

edit: Rules do not seem to provide a drop-in alternative: triggers seem to be based on sensor values 'onchange' rather than 'onset' behavior.

uzi18 commented 4 years ago

you can trigger timer every 10s. if you want and send all needed vars with sendtohttp at once

pkoster commented 4 years ago

Considering the likely intended behavior of Generic HTTP, what about a modification like:

diff --git a/src/_C008.ino b/src/_C008.ino
index 902e1988..a3509ad8 100644
--- a/src/_C008.ino
+++ b/src/_C008.ino
@@ -49,17 +49,23 @@ bool CPlugin_008(CPlugin::Function function, struct EventStruct *event, String&

     case CPlugin::Function::CPLUGIN_PROTOCOL_SEND:
       {
+        MakeControllerSettings(ControllerSettings);
+        LoadControllerSettings(event->ControllerIndex, ControllerSettings);
+        String publish(ControllerSettings.Publish);
+
         // Collect the values at the same run, to make sure all are from the same sample
         byte valueCount = getValueCountFromSensorType(event->sensorType);
+        if ( publish.indexOf(F("%valname%")) == -1 && // iterate over values with multiple
+             publish.indexOf(F("%value%")) == -1 &&   // HTTP requests if it would generate
+             valueCount != 0 ) {                      // different requests (= original
+          valueCount = 1;                             // behavior); otherwise, send one HTTP
+        }                                             // request (= new behavior)
         C008_queue_element element(event, valueCount);
         if (ExtraTaskSettings.TaskIndex != event->TaskIndex) {
           String dummy;
           PluginCall(PLUGIN_GET_DEVICEVALUENAMES, event, dummy);
         }

-        MakeControllerSettings(ControllerSettings);
-        LoadControllerSettings(event->ControllerIndex, ControllerSettings);
-
         for (byte x = 0; x < valueCount; x++)
         {
           bool isvalid;

This preserves the behavior of multiple HTTP requests where it has meaningful effect because the requests potentially differ (%valname% and %value% iterating over the values of the different variables), but avoids where it has no effect and requests would just be duplicates (in absence of %valname% and %value%).

TD-er commented 4 years ago

It is not the same behavior, as there is also the functionality where other variables are replaced. I think it would make more sense to add a new parameter to force the number of parameters to N. For example based on set IDX, which is not used in this controller, but can be set at the plugin side.

uzi18 commented 4 years ago

we talked about such case in time when we want to add nettemp controller (about 2 years ago?), we were forced to use HTTP Advanced controller (+wiki description), but it not resolved special case as nettemp server can fetch all variables in one request, cant find this discussion. Plus one special case if still remember - it was switch/gpio or something like that, so we need to use 2 times http adv. controller.

resolved most of these problems by unofficial build with nettemp controller

PR was here: #143

tonhuisman commented 10 months ago

This seems to be resolved, can most likely be closed.