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

Don't attempt to get local IP address if it's already specified on command line #11

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

srd424
Copy link
Contributor

@srd424 srd424 commented Nov 4, 2024

For some reason, the attempt to get the local IP address hangs in one of my containers. This patch at least stops the code trying to fetch it if it's already specified on the command line. This is mostly whitespace/indent change. Do check it because I don't really know js!

Copy link
Owner

@suletom suletom left a comment

Choose a reason for hiding this comment

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

Hi!

Strange behavior. The fix seems good, absolutely no need to wait for ip if it's already known, only i needed the callback because i didn't want to use async for some reason, don't know whether this solution has side effects. Check this later....

But if you have some time, please help me tracking the reason for the original issue.
The used module only does really basic things to determine ip: https://github.com/Aldaviva/local-ipv4-address/

According to the module source i guess the following process calls needs investigation inside the container:
on linux: netstat -r -n -A inet
on windows: route print -4 0.0.0.0

@srd424
Copy link
Contributor Author

srd424 commented Nov 7, 2024

Yeah, it was weird, I had a poke around with strace, ran the commands manually, couldn't figure out what was going on. It's also possible I got confused of course (particularly by the async stuff), I will try to remember to have another look!

FWIW, netstat and ifconfig (which that module also uses, I think) are both fairly old fashioned / deprecated on Linux now, you'll find that lots of environments don't include them by default (though mine do - as I am also old-fashioned!) ss(8) is the modern equivalent to netstat, and ip(8) replaces ifconfig, though it should also be possible to read a lot of that information directly out of files in /proc or /sys, I would think.

@suletom
Copy link
Owner

suletom commented Nov 7, 2024

Hi,

I'm old fashioned linux guy too, but can confirm that module is older than 7 years, the only reason i choosed it, was to query the ip associated with the default route. As i see other modules do similar hacks, for example this:
https://www.npmjs.com/package/default-gateway
I guess the inbult "os" module interface lacks these features.

Your solution is a good way to go, but let me investigate later because i'm busy with other work nowadays.

BTW i'm using the web version of the script from the "dev" branch: https://github.com/suletom/EASUN-ISOLAR-SMX-II-CONTROL/blob/dev/demo.png
Lot of cool features has been implemented on this (chart, monitoring, ups style energy management with solar prediction), but it also lacks some other functions, for example "plug pro" related things. Unfortunately lot of fixes needs to be merged to this branch too, but my plan is to add multiple device support to this version at the future.

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.

2 participants