add support for custom regions, by detecting what is used in index.html

[sdetweil]

read index.html to discover the regions used, make them the list checked by app.js and check:config test

fixes #3504 supercedes #3506

no config.js param required

[sdetweil]

how do I create a testcase where I need to replace index.html??
I need 3 testcases

1. bad position name (standard index.html)
2. custom position, config matching, need new index.html
3. custom position, config wth new position mispelled, for error of new, need new index.html (same as test 2)

I removed the hard coded position names from the code... all is detected

[khassel]

In e2e weather tests is a similar construction where a string #####WEATHERDATA##### is replaced in config.js, you can search for weather_mocker in the test folder.

I think you have to restore the original index.html after your tests (e.g. bei calling git checkout index.html).

[sdetweil]

that replaces the string in the config.js
I understand I would have to restore it.. just haven't seen a file replace as we can specify where the config is thru env vars...

[khassel]

yes, we have no such file replace, maybe simple solution is

• load file content
• replace (e.g. valid position with invalid)
• save changed file content
• test
• revert changes

[khassel]

supercedes #3506

what about the changes done in the above PR? Do we still need allowCustomModulePositions or should that commit be reverted?

[sdetweil]

i removed it.. (removed the code, not the commit)

still testing.. testcase fails.. not sure why..

[sdetweil]

ok.. one more problem on this

so I filtered the regions to positions and we went thru config , and checked.. all good..

start server, open web window, on index.html..
main.js loads all the module js files.. oops.. it has a hard coded list of positions.. doah..

so, i have to pass the generated list
include another js file for it to load?
which has the array definition of the found positions?

this works..

new file
js/positions.js
const modulePositions = [......]

comment out the same in main.js

any objections to this approach?
rewrite the js/positions.js file on each startup (ugh)

[sdetweil]

is there a testcase that captures error

-08-04 15:57:54.546] [WARN]  Invalid module position found for this configuration:
{
  "module": "helloworld",
  "disabled": false,
  "position": "row3_left1",

as the position is bad, MM doesn't load the module, there is no web content.. so we wait til timeout..

[khassel]

is there a testcase that captures error

I think no looking at #3445

[khassel]

any objections to this approach?

not sure, I did similar ugly things in https://github.com/MagicMirrorOrg/MagicMirror/pull/3231/files replacing a placeholder with content of a config property

[sdetweil]

ok.. I will be gone for a week (mon-mon) so won't work on it til I get back.. just have to figure out verifying the bad position requested.. have good now... I will push all this so not lost

[sdetweil]

this is ready for review

[sdetweil]

what is this failure? I didn't change anything in the on/off rebuild test

[khassel]

and I think allowCustomModulePositions: false, should be removed from js/defaults.js

[sdetweil]

ok missed that.

[sdetweil]

ok, updated to remove that

[bugsounet]

@sdetweil : I just discover a bug in getModulePositions()

This function is called 2 times in app.js

• 1st time in start of MM with require(${global.root_path}/js/check_config.js); (L120)
• 2nd time in this.start with Utils.getModulePositions() (L272)

Result:
modulePositions have duplicate positions and write the positions.js file 2 times

just check position.js file
const modulePositions=["fullscreen_below","top_bar","top_left","top_center","top_right","upper_third","middle_center","lower_third","bottom_bar","bottom_left","bottom_center","bottom_right","fullscreen_above","fullscreen_below","top_bar","top_left","top_center","top_right","upper_third","middle_center","lower_third","bottom_bar","bottom_left","bottom_center","bottom_right","fullscreen_above"]

[sdetweil]

thats because check:config was just added to the startup.

[bugsounet]

I know sam, i'm just reporting what I've noticed.

Maybe removing L272 and using only check config will do the job

[sdetweil]

i didn't add check config.

[bugsounet]

You have right sam,

it's @KristjanESPERANTO #3450

[sdetweil]

or we could add a check in getModulePositions if the modulePositions is not populated

[sdetweil]

i added a pr for the latter, check if work already done, in case we remove check:config from startup

[bugsounet]

it seems to do the job ;)