Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix variable problems #3304

Merged
merged 1 commit into from
Dec 28, 2023
Merged

fix variable problems #3304

merged 1 commit into from
Dec 28, 2023

Conversation

khassel
Copy link
Collaborator

@khassel khassel commented Dec 25, 2023

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

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

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

Copy link
Collaborator

@sdetweil sdetweil left a comment

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
6 checks passed
@khassel khassel deleted the electron-port branch December 28, 2023 22:54
@BKeyport
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants