Split up cmdtbl even further because almost 64k are reached

[fredlcore]

It's quite impressive that not even two years ago, I had to split up the cmdtbl struct in two pieces because it went beyond the 32kB limit that exists on the Arduino Mega. Now both cmdtbl1 and cmdtbl2 each reach 32kB, meaning the number of supported parameters we found for various heating systems has doubled, which nicely shows the collaborative effort and success of this project. We now have to think about whether we should make a departure here from the Mega and stop splitting up cmdtbl further, or if we make the effort and split cmdtbl into three parts. This will most likely be the last time we would have to do this because once we have filled another 32kB of parameter data, the memory of the Mega 2560 will be maxed out anyway. But the way the split up is handled now is quite inefficient (code-wise), as I had to do it in a hurry back then. It probably would be worth it to do the calculation in just one function and not do it each time in each get_cmdtbl_xxx function.

What do you guys think?

[fredlcore]

I just realized that outsourcing the calculation is not enough because we still have to identify whether cmdtbl1, cmdtbl2 or cmdtbl3 has to be used. Not sure if we can do another function that returns a reference to the determined cmdtblX. Returning both the index and the reference is not possible (or reasonable), I guess...

[hacki11]

What about a index-table containing all command IDs and a pointer to the entry in cmdtbl<X>? Would that fit into 32kB? The command ID from cmdtbl<X> can then be removed

[fredlcore]

That would mean more effort when adding a new parameter (or a specific version for a heating system for a parameter) because I would have to adjust the data in two places instead of one. Removing the commandID from the struct would save 4 of 17 bytes, so a bit less than 25%. Currently, we have close to 64kB in data, 25% less would still be more than 32kB.

[dukess]

I think that we can add one more table.

[fredlcore]

With the addition of another 100 entries in cmdtbl, we will now have to split it into three parts, otherwise it won't compile on the Mega anymore. @dukess, you said, you tried to do that, but in the end, it didn't work on the Mega? Should we give it another try or drop Mega support at this stage? It doesn't make sense to now start to filter parts of the cmdtbl via "#ifdef" etc. because that would make maintaining the table even uglier ;)...

[dukess]

Now we have three tables and it works so you can add new lines. I have radical idea for Mega users: we can make script which will generate cmdtbl special for selected device family and model.

fredlcore notifications@github.com 31 декабря 2020 г. 1:55:05 написал:

With the addition of another 100 entries in cmdtbl, we will now have to split it into three parts, otherwise it won't compile on the Mega anymore. @dukess, you said, you tried to do that, but in the end, it didn't work on the Mega? Should we give it another try or drop Mega support at this stage? It doesn't make sense to now start to filter parts of the cmdtbl via "#define" etc. because that would make maintaining the table even uglier ;)... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[dukess]

Prototype. Need to move tables content to cmdtbl.txt before start. Result will be placed in result1.txt TO DO: include result1.txt into source Usage

./table_generator DEVICE_FAMILY DEVICE_MODEL

#!/bin/bash
if [ "${1}" == "" ]; then
  FAMILY="ALL"
else
  FAMILY=${1}
fi

if [ "${2}" == "" ]; then
  DEVICE="ALL"
else
  DEVICE=${2}
fi

if [[ "${1}" == "" && "${2}" == "" ]]; then
GREPSTR='({.*,.*,.*,.*})'
else
GREPSTR="(DEV_ALL|DEV_${FAMILY}_ALL|DEV_${FAMILY}_${DEVICE})";
fi

maximum_tables=3;
total_count=`cat cmdtbl.txt | grep -c -E "${GREPSTR}"`
let table_count=total_count/maximum_tables+10;
let i=0;
let j=1;
echo -n "" > result1.txt

cat cmdtbl.txt | grep -E "${GREPSTR}" | while IFS= read -r line; do
if [ ${i} == 0 ]; then
echo "PROGMEM_LATE const cmd_t cmdtbl$j[]={" >> result1.txt
let j=j+1;
fi

let i=i+1;

if [ ${i} == ${table_count} ]; then
echo "${line}" | sed "s/},/}/" >> result1.txt
echo "};" >> result1.txt
i=0;
else
echo ${line} >> result1.txt
fi
done
echo "};" >> result1.txt

[1coderookie]

I have radical idea for Mega users: we can make script which will generate cmdtbl special for selected device family and model.

That idea is really cool and could work great if one uses the adapter at only one controller. But: We also have cases where the adapter is connected via LPB and has access to more than one controller type (means different controller family&variant), so maybe there could occur some problems..? Just wanted to mention that scenario, so that you guys have it in mind..

[fredlcore]

Yes, Ulf is right, that would be a problem (and not just for LPB, in case someone wants to set/query other BSB-connected devices). The problem I see is that most users run Windows, so it will be more difficult to create a script that runs out-of-the-box. Maybe it's easier just to take each line from BSB_landefs.h and drop every line that contains DEV\d\d\d where \d\d\d is not equal to one's own device family?

As for the output, I would name the file "BSB_lan_defs_xxx_yyy.h" and then users just need to change the include statement in BSB_lan.ino, or they remove the full BSB_lan_defs.h and rename the generated file.

[dukess]

to more than one controller type

We can set multiple families and devices with comma and parse these strings and prepare regular expression.

Maybe it's easier just to take each line from BSB_landefs.h and drop every line that contains DEV\d\d\d where \d\d\d is not equal to one's own device family?

BSB_lan_defs.h also contain other definitions and parsing will be more complex. In any case main difficulty is most users will use Windows. As idea: create online service with table generator.

[fredlcore]

Yes, an online table generator could be an option.

As for other definitions: I was thinking of something like this:

1. Read line from BSB_lan_defs.h
2. Test if line matches DEV_\d\d\d
3. If no, write line to output file, continue at 1.
4. If yes, test if line matches DEV_XXX where XXX is the own device family.
5. If it matches, write to outputfile, otherwise discard.

That way the whole BSB_lan_defs.h gets copied and only those lines are dropped which are not DEV_XXX.

[fredlcore]

Here's a script that should do the trick:

#!/usr/bin/perl

open (DEFS, "BSB_lan_defs.h");
while ($line = <DEFS>) {
  if ($line !~ /, *DEV\_\d\d\d\_/) {
    print $line;
  } else {
    foreach $devfam (@ARGV) {
      if ($line =~ /, *DEV\_$devfam\_/) {
        print $line;
      }
    }
  }
}

./selected_defs.pl 162 165 would output the whole _defs.h except all device family/variant specific lines, unless they are for device families 162 and 165.

[fredlcore]

Added as selected_defs.pl to the repo.

[dukess]

Nice gift for new year!

fredlcore notifications@github.com 31 декабря 2020 г. 17:07:15 написал:

Added as selected_defs.pl to the repo. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[fredlcore]

It was your great idea - should have thought about this long time ago already :)... And thanks, Ulf, for pointing out the issue with the several possible device families in one setup, didn't think of that either...

[1coderookie]

You're welcome - glad that I could be helpful this time :) I'll add your description from the forum to appendix D soon (but probably not until next year ;) ), to keep the infos for mega users pretty much at one place.

[fredlcore]

@1coderookie: Thanks! @all: I have added a Windows executable of the Perl script, so there is no need for an online service anymore :).

[dukess]

Great!

fredlcore notifications@github.com 1 января 2021 г. 18:07:26 написал:

@1coderookie: Thanks! @ALL: I have added a Windows executable of the Perl script, so there is no need for an online service anymore :). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.