[FEATURE REQUEST] Streamline accessing multiple devices on the bus

[fredlcore]

Since there are more and more people who have more than one device on the (LPB-)bus who also want to access parameters from these devices, we have previously implemented the possibility to temporarily set a different device as destination by using the exclamation mark followed by the device ID, such as /8700!1. This works fine via the web-interface and implementations which use the web-interface for access. However, it does not work when using MQTT or when using /JQ or /JS. Since more and more users use a home automation system that relies on MQTT or JSON, we should act in this regard. The question is whether it is easier to parse the parameter number for the exclamation mark also in the MQTT/JSON part of the code (the same way we do it when parsing the URL string – or whether we should look at a different solution alltogether, even if it might mean moving eventually away from the exclamation mark approach. Using the exclamation mark across all functions might mean quite some changes where the parameter number is not stored as a string, but I haven't fully checked this.

What do you think, @dukess, @Luposoft63, @mirkolenz and others?

[dukess]

I think we can add one more field for JSON, something like "address". If "address" value is not presented then we will use default address which setup in configuration page. In other case we will use temporary destination address.

fredlcore @.***> 18 января 2023 г. 21:51:39 написал:

Since there are more and more people who have more than one device on the (LPB-)bus who also want to access parameters from these devices, we have previously implemented the possibility to temporarily set a different device as destination by using the exclamation mark followed by the device ID, such as /8700!1. This works fine via the web-interface and implementations which use the web-interface for access. However, it does not work when using MQTT or when using /JQ or /JS. Since more and more users use a home automation system that relies on MQTT or JSON, we should act in this regard. The question is whether it is easier to parse the parameter number for the exclamation mark also in the MQTT/JSON part of the code (the same way we do it when parsing the URL string – or whether we should look at a different solution alltogether, even if it might mean moving eventually away from the exclamation mark approach. Using the exclamation mark across all functions might mean quite some changes where the parameter number is not stored as a string, but I haven't fully checked this.What do you think, @dukess, @Luposoft63, @mirkolenz and others? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

[dukess]

Oh, sorry, i forgot about "Destination". It already allowing send telegrams to selected address.

fredlcore @.***> 18 января 2023 г. 21:51:39 написал:

Since there are more and more people who have more than one device on the (LPB-)bus who also want to access parameters from these devices, we have previously implemented the possibility to temporarily set a different device as destination by using the exclamation mark followed by the device ID, such as /8700!1. This works fine via the web-interface and implementations which use the web-interface for access. However, it does not work when using MQTT or when using /JQ or /JS. Since more and more users use a home automation system that relies on MQTT or JSON, we should act in this regard. The question is whether it is easier to parse the parameter number for the exclamation mark also in the MQTT/JSON part of the code (the same way we do it when parsing the URL string – or whether we should look at a different solution alltogether, even if it might mean moving eventually away from the exclamation mark approach. Using the exclamation mark across all functions might mean quite some changes where the parameter number is not stored as a string, but I haven't fully checked this.What do you think, @dukess, @Luposoft63, @mirkolenz and others? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

[dukess]

Using syntax like /JQ=7800!1,1602,700!3 - good idea. Parser will be have compact implementation and and allow to use current user experience.

fredlcore @.***> 18 января 2023 г. 21:51:39 написал:

Since there are more and more people who have more than one device on the (LPB-)bus who also want to access parameters from these devices, we have previously implemented the possibility to temporarily set a different device as destination by using the exclamation mark followed by the device ID, such as /8700!1. This works fine via the web-interface and implementations which use the web-interface for access. However, it does not work when using MQTT or when using /JQ or /JS. Since more and more users use a home automation system that relies on MQTT or JSON, we should act in this regard. The question is whether it is easier to parse the parameter number for the exclamation mark also in the MQTT/JSON part of the code (the same way we do it when parsing the URL string – or whether we should look at a different solution alltogether, even if it might mean moving eventually away from the exclamation mark approach. Using the exclamation mark across all functions might mean quite some changes where the parameter number is not stored as a string, but I haven't fully checked this.What do you think, @dukess, @Luposoft63, @mirkolenz and others? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

[fredlcore]

Thanks, ok, so that means that setting the destination address is already possible with JSON when using POST data? Only for writing or also for reading? In my opinion, there is no need to implement /JQ=xxxx!y if /JQ works with POST data and then using destination:. Using /JQ via the URL was only an accidental feature in the first place. Unless the changes to include that are minimal.

[dukess]

Parameter writing will works exactly, but for reading an attempt to send the destination address via POST will lead to unpredictable behavior. /JQ can't use right now parameter numbers from JSON

[mirkolenz]

This is quite an interesting topic. I was not aware of the fact that it is even possible to use this exclamation mark to address separate devices on the bus. I only worked with the MQTT interface, so I cannot comment on JSON. Since the MQTT interface basically resembles the URL scheme when getting or setting params, I think it makes sense to stick to the exclamation mark.

However, I also think that the MQTT interface currently is not very intuitive to use. Native MQTT operations would normally use addresses like 8700/get or 8700/set instead of writing the required URL to the main topic. So if this project may move in such a direction for MQTT, then one could also include the device id in the address (e.g., 8700/get/1 instead of /8700!1).

To summarize: If you plan to stick to the URL scheme for MQTT, the exclamation mark is perfectly fine. In case a larger refactoring would be done, then the device id could be incorporated in better ways.

[fredlcore]

Sticking to the exclamation mark will at least provide a problem for parameter values that are sent to the broker during the log interval. These parameters are stored as floats currently, so adding an exclamation mark in the (web-)configuration will not work at the moment :(.

[fredlcore]

I have now found a way to stick to the exclamation mark in the webinterface by parsing the parameters and storing the destination after the exclamation mark in a separater position in the array. That also means that each log parameter in BSB_LAN_config.h now consists of two values: One for the parameter and one for the destination, like this:

float log_parameters[40] = {
// paramater, destination
  8700, 0,                  // Außentemperatur
  8326, 1                   // Brenner-Modulation
};

@dukess: This effectively reduces the number of possible logging parameters from 40 (the current size of the array) down to 20 (because each parameter takes two values: parameter number and destination). Can I safely increase the array size from 40 to 80? Or will this affect the EEPROM layout? I assume it will. But how do I bump up the EEPROM version in order to recreate the EEPROM layout without adding a new configuration option?

Other than that, it's working fine and both logging to SD card as well as sending parameters with a different destination to the MQTT broker works fine.

[fredlcore]

I have now uploaded the changes to GitHub, maybe someone using MQTT can try especially the set function via MQTT. Thanks!

[mirkolenz]

My heater only has a single bus device, so I cannot help test it. But it would still be interesting to know if this change reduced the number of possible logging parameters :)

[fredlcore]

I have now installed a mosquitto server and tried setting as well as querying values. Works fine. Also tested querying/setting parameters using JSON and the destination key. Works as well (as @dukess already mentioned). And since it seems to be used quite often, I have also added support for specifiying a destination for /JQ in the URL as well. So /JQ=710!1,712!1 will now work as well. The same goes for querying a category via /Kx!y - here you can also specify the destination y for category x.

[dukess]

Wow, that's cool!

I can suggest some improvements (which, as usual, break compatibility): I suggest replacing the float array with an array of structures:

typedef struct {
  float number;
  byte dest_addr;
} program;

In this case, the definition of array of programs will look like this: program log_parameters[40] = {{8700}, {8326, 1}}; "Contra": we should to change the value parser for the configuration page (and somewhere else). "Pro": this type of data can be used everywhere in the program. There is less chance of error when setting values. The code will be more readable. Lesser memory consumption.

P.S. Sorry for the absence: I had a lot of interesting work.

[dukess]

https://github.com/dukess/BSB-LAN/commit/f449ec00edc3d8d61d477c231414a15748909bd8 Some improvements: Code should be smaller. log_averages, rgte_sensorid, ipwe_parameters, mqtt_callback can query temporary destination too.

[fredlcore]

Good to hear you are doing fine - as for the array, I realize that it would improve readability, but having to rewrite the parser and for users to adjust their _config.h again after they just had to change it last week is probably not worth the effort.

But what do you mean with

Code should be smaller. log_averages, rgte_sensorid, ipwe_parameters, mqtt_callback can query temporary destination too.

Except for maybe rgte_sensorid all others should be able to query temporary destinations, too.

[dukess]

Except for maybe rgte_sensorid all others should be able to query temporary destinations, too.

My mistake. I improved log_averages and ipwe_parameters:

BSB_LAN.ino: Line 802 - bug (array overflow) Lines 7111-7112 will always executed without assign temporary address. include/print_ipwe.h: Line 23 - bug (array overflow) Lines 28-29 will always executed without assign temporary address.

[fredlcore]

Hm, could you check the line numbers again? Neither in your commit nor in the current master branch do I see something related to a temporary address? If the bugs are still there, could you do a PR first for just the fixes before anything else? Thanks!

[dukess]

Line 23 - bug (array out-of-bound) https://github.com/fredlcore/BSB-LAN/blob/4ae0c1288d5c46b40672a7e0593b979a052e95bd/BSB_LAN/include/print_ipwe.h#L23 Line 802 - bug (array out-of-bound) https://github.com/fredlcore/BSB-LAN/blob/4ae0c1288d5c46b40672a7e0593b979a052e95bd/BSB_LAN/BSB_LAN.ino#L802 They should be as here (divided by 2): https://github.com/fredlcore/BSB-LAN/blob/4ae0c1288d5c46b40672a7e0593b979a052e95bd/BSB_LAN/BSB_LAN.ino#L807

Lines 28-29 will always executed without assign temporary address. https://github.com/fredlcore/BSB-LAN/blob/4ae0c1288d5c46b40672a7e0593b979a052e95bd/BSB_LAN/include/print_ipwe.h#L27-L29

Ok, i will try to do little bit later, because the patch that I did today needs to be put aside somehow for the future and start working from scratch. :)

I'm amazed at what you guys have done, but I don't like the fact that additional arithmetic operations are now required to recalculate positions in arrays. Errors will definitely appear in these places.

[fredlcore]

Yes, you are right, and I'm certainly open to implement your solution at a later stage when we have the need to change BSB_LAN_config.h next time or with the next other kind of breaking change.

[dukess]

I had another thought: there are conditions in the code now that are triggered if the temporary address is greater than zero. But at the same time, zero is also the correct address on the bus, and such a restriction can lead to unpredictable consequences for users if our "own_address" parameter has a value other than 0. I think it makes reason to use "-1" as the "default address" value in arrays.

[fredlcore]

Good call, I'll have to take a look at that. I got the 0 confused with the RX/TX pins where 0 is the auto-detection, I guess...

[fredlcore]

Hm, I have just searched for the conditions, and the only > 0 conditions I found are meant to check if a parameter has been set (which means you can't log parameter 0), as in the first line of the following code snippet. Regarding the temporary destination, I used code like in the second line:

         if (log_parameters[i*2] > 0) {
            if (log_parameters[i*2+1] != d_addr) {
              d_addr = log_parameters[i*2+1];
              printFmtToDebug(PSTR("Setting temporary destination to %g\r\n"), d_addr);
              bus->setBusType(bus->getBusType(), bus->getBusAddr(), (uint8_t)d_addr);
              GetDevId();
            }
            mqtt_sendtoBroker(log_parameters[i*2], log_parameters[i*2+1]);  //Luposoft, put whole unchanged code in new function mqtt_sendtoBroker to use it at other points as well
          }
        }

However, I realized (apart from several places where I forgot to multiply by two) that the temporary destination is not taken care of at various places, most notably with avg_parameters[], e.g. storing average parameters on SD card or displaying parameters on the IPWE page. This still needs to be taken care of, and the question is how to do this best. On simple output, like IPWE, we can just add the ! after the parameter number.

But when the list of average parameters is output via JSON, 8310!1 is not a number anymore, so that will be a problem.

And I have thought about your solution again, @dukess, and maybe we can find a good compromise, because I realize now that this multiplication thing is really not feasible. But for the sake of the users and easier input (not so many curly brackets that aren't even printed on my keyboard), could we do it like this? We keep the way log_parameters[] and the others are written, i.e. just a comma separated array which you then read into your struct and only use the struct in the code? Worst case, if we cannot destroy the original log_parameters[] array and free up its memory would be a waste of 80 bytes per array. Would be worth it, in my opinion.

We might even be able to keep the parser because the notation with the exclamation mark is already known to users. We would still need to find a way for the abovementioned issues like how to display the destination in the JSON output for the averages etc., but it still would be easier and cleaner regarding the code. I would just rename the struct from program to parameter because that is how we usually refer to.

And one thing that needs to be checked in set_temp_dest is whether newDestAddr is already equal to the current temporary address (getBusDest()) so that calls to several parameters to the same temporary destination do not result in a query to 6225/6226 each time but just once until the temporary destination changes again.

[dukess]

The format for entering the parameter number in the form xxxxx!yyy seems to me convenient and universal. We can make one recognition function for it, and this format can be used everywhere: in the URL, web configurator, MQTT.

In JSON, I suppose we will have to add a new field "dest_addr", which will be displayed if the request was made at a temporary address. By the fact, parameter number in JSON is a string (printFmtToWebClient(PSTR(" \"%g\": {\r\n \"name\": \""), json_parameter);), so we can add suffix with destination address.

An idea came to my mind: what if, when starting the device or changing the bus, scan the network for the presence of devices (as the /Q function does) and put the found addresses and family/model in a table? "Pro" : Switching between devices will be faster. "Contra": additional memory will be required, the launch will take longer. To speed up the restart, it makes sense to consider saving the "network map" in the EEPROM.

I need to think about the idea of abandoning curly braces and using intermediate arrays. (You have a very interesting keyboard! :) )

[fredlcore]

As for JSON, we already have the "destination" field which works both in /JQ and /JS, or where else do you think we would need this? I also had the idea of scanning the bus in the beginning. The thing is that with the device specific parameter list, this won't work on all devices on the bus which show up during the scan (for example the display unit on ID 10) and thus might confuse users. Also, the device specific parameter list is always based on the categories from device ID 0 (because there might be overlappings and other inconsistencies between the various devices on the bus). So offering the destination might give the impression that everything will be adjusted to that specific (temporary) destination. But since it's not, it might be better if users have to know what they are doing by explicitly calling the parameters or categories for that (temporary) destination. But I'm not against offering such a selection, it's just that it might not always offer what people would be expecting from it.

And yes, Macbook keyboards have very little extra signs on their keyboards (and use weird key combinations to access them), so I have to press three keys to print a backslash which isn't even printed on the corresponding key ;)...

[dukess]

Perhaps these arguments are sufficient to avoid doing bus scanning functionality now.

In the new MR, I wanted to make only a general function for parsing the string, but this entailed changes in the destination change code. https://github.com/fredlcore/BSB-LAN/pull/572 At the moment, MR is intended only for review, since I haven't tested it on the MCU yet.