Skip to content

fix variable problems#3304

Merged
rejas merged 1 commit intoMagicMirrorOrg:developfrom
khassel:electron-port
Dec 28, 2023
Merged

fix variable problems#3304
rejas merged 1 commit intoMagicMirrorOrg:developfrom
khassel:electron-port

Conversation

@khassel
Copy link
Collaborator

@khassel khassel commented Dec 25, 2023 •
edited
Loading

related to #3302

  • fix MM_PORT variable not used in electron
  • fix to allow full path for MM_CONFIG_FILE variable

@sdetweil
Copy link
Collaborator

sdetweil commented Dec 25, 2023 •
edited
Loading

when I do this , in ~/MagicMirror/mm.sh

export MM_CONFIG_FILE=/home/sam/MagicMirror/config/config.js
export MM_PORT=8090
echo $MM_CONFIG_FILE
npm start

I get the mm error screen
Screenshot at 2023-12-25 14-39-19

and when I nano that path
/home/sam/MagicMirror/config/config.js
I see
Screenshot at 2023-12-25 14-42-17

@sdetweil
Copy link
Collaborator

sdetweil commented Dec 25, 2023

i added a config.log() after resolving path to config file in app.js
it completes with correct name
and loads correct module node helpers, but gives the error screen to browsers... (on the specificed port in MM_PORT)

[25.12.2023 14:51.53.887] [LOG]   Starting MagicMirror: v2.25.0
[25.12.2023 14:51.53.889] [LOG]   Loading config ...
[25.12.2023 14:51.53.890] [LOG]   global config file=/home/sam/MagicMirror/config/config2.js
[25.12.2023 14:51.53.891] [DEBUG] config template file not exists, no envsubst
[25.12.2023 14:51.53.893] [LOG]   Loading module helpers ...
[25.12.2023 14:51.53.896] [LOG]   No helper found for module: alert.
[25.12.2023 14:51.54.108] [LOG]   Initializing new module helper ...
[25.12.2023 14:51.54.109] [LOG]   Module helper loaded: MMM-ImagesPhotos
[25.12.2023 14:51.54.109] [LOG]   No helper found for module: clock.
[25.12.2023 14:51.54.177] [LOG]   Initializing new module helper ...
[25.12.2023 14:51.54.178] [LOG]   Module helper loaded: calendar
[25.12.2023 14:51.54.336] [LOG]   Initializing new module helper ...
[25.12.2023 14:51.54.336] [LOG]   Module helper loaded: MMM-Config
[25.12.2023 14:51.54.336] [LOG]   No helper found for module: compliments.
[25.12.2023 14:51.54.378] [LOG]   Initializing new module helper ...
[25.12.2023 14:51.54.378] [LOG]   Module helper loaded: newsfeed
[25.12.2023 14:51.54.378] [LOG]   All module helpers loaded.
[25.12.2023 14:51.54.382] [LOG]   Starting server on port 8091 ... 
[25.12.2023 14:51.54.384] [WARN]  You're using a full whitelist configuration to allow for all IPs
[25.12.2023 14:51.54.390] [LOG]   Server started ...
[25.12.2023 14:51.54.390] [LOG]   Connecting socket for: MMM-ImagesPhotos
[25.12.2023 14:51.54.390] [LOG]   Starting node helper for: MMM-ImagesPhotos
[25.12.2023 14:51.54.390] [LOG]   Connecting socket for: calendar
[25.12.2023 14:51.54.390] [LOG]   Starting node helper for: calendar
[25.12.2023 14:51.54.391] [LOG]   Connecting socket for: MMM-Config
[25.12.2023 14:51.54.391] [LOG]   Starting module helper: MMM-Config
[25.12.2023 14:51.54.391] [LOG]   Connecting socket for: newsfeed
[25.12.2023 14:51.54.391] [LOG]   Starting node helper for: newsfeed
[25.12.2023 14:51.54.391] [LOG]   Sockets connected & modules started ...
[25.12.2023 14:51.54.581] [LOG]   Launching application.

@khassel
Copy link
Collaborator Author

khassel commented Dec 25, 2023

can not reproduce your errors, with this fix the port variable is working with electron and also in browser.

I used the sample config.js and only changed address: "0.0.0.0", and ipWhitelist: [],

@rejas
Copy link
Collaborator

rejas commented Dec 27, 2023

so, can we merge it or not? dont want to decide since I dont have time for testing myself

@khassel
Copy link
Collaborator Author

khassel commented Dec 27, 2023

will update this later with another fix

@khassel khassel marked this pull request as draft December 27, 2023 10:03
@khassel khassel changed the title fix MM_PORT variable not used in electron fix variable problems Dec 27, 2023
@khassel
Copy link
Collaborator Author

khassel commented Dec 27, 2023

I added another fix which I think was the main problem because using a full path for MM_CONFIG_FILE did not work.

So @sdetweil @rejas feel free to test again, from my side this can me merged.

@khassel khassel marked this pull request as ready for review December 27, 2023 10:27
@rejas rejas requested a review from sdetweil December 27, 2023 10:31
@rejas
Copy link
Collaborator

rejas commented Dec 28, 2023

would like to see a test&approval by @sdetweil since I dont have much time these days

sdetweil
sdetweil approved these changes Dec 28, 2023
View reviewed changes
Copy link
Collaborator

@sdetweil sdetweil left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is tested and works fine...

the doc needs to have a statement that the targeted config file must reference a filename in the MagicMirror folder tree (that filename could be linked to a physical file anywhere in the filesystem, on linux systems see the ln command)

@khassel
Copy link
Collaborator Author

khassel commented Dec 28, 2023

the doc needs to have a statement that the targeted config file must reference a filename in the MagicMirror folder tree (that filename could be linked to a physical file anywhere in the filesystem, on linux systems see the ln command)

will take a look at the docs after this is merged

@rejas rejas merged commit a927eb2 into MagicMirrorOrg:develop Dec 28, 2023
@khassel khassel deleted the electron-port branch December 28, 2023 22:54
@BKeyport
Copy link
Contributor

BKeyport commented Dec 29, 2023

Spotted and modified doc re-write to match Sam's note.

@BKeyport BKeyport mentioned this pull request Dec 29, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rejas rejas rejas approved these changes

@sdetweil sdetweil sdetweil approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@khassel @sdetweil @rejas @BKeyport