-
Notifications
You must be signed in to change notification settings - Fork 75
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
(macOS) seqbuffer.js FIX static IP handling via config file #58
base: master
Are you sure you want to change the base?
Conversation
*just fine tuning
*Ignore, OFFER message order correction while trying to look why Microsoft devices fails read it correctly.
Typo
Typo: doesn't work for me with : when running as server and i boot up any device. - style works for me but i am using randomIP: false as well if it matters.
noobie didn't find revert changes button.. i found source of why Static mac addresses with ':' marks doesn't work
noobie didn't find revert changes button.. i found source of why Static mac addresses with ':' marks doesn't work
Real source why MAC addresses used at static ip assignments didn't work with ':' style but instead '-' at examples/server.js and console.log prints MAC addresses with '-' instead ':' as should be for more humans. If there was some reason to use '-' instead of ':' i am CLEARLY not aware of it.
Real source (at least for me..) why MAC addresses used at static ip assignments didn't work with ':' style but instead '-' at examples/server.js and console.log now prints MAC addresses with ':' instead earlier '-' as should be for more humans. If there was some reason to use '-' instead of ':' i am CLEARLY not aware of it.
Sorry, I won't merge your pull request into master branch. You hard-code a bunch of options, but what about all the others? And the MAC-separator is OS-dependent, means that with your change Linux/OSX does not work anymore. |
Ah okies i see. I am running (not called OSX anymore) macOS Sierra which it fails on if try go with examples. Just confusing as its there hard coded to use hyphens '-' but in example it is shown to user to use as ':' (colons) and it doesn't work for me if follow that fully on macOS which i am running all. hyphens - or colons : The standard (IEEE 802).. But i saw you had hard coded tests to use hyphens (tests/segbuffer.test.js) which fails just and not OS had anything to do with it i assume because both works here if only i fix everything to use the same style but there is tried to use two styles in whole dhcp which leaves it partially broken or is misleading. So my main point is to fix it to use hyphens fully or colons because if i pass MAC address with colons on macOS it doesn't understand because it is FORCED to check it against FIXED hyphens so no matter what i input the result is the same = FALSE or not found static ip in translated. Here is one hard coded test with hyphen style instead like in example which shows to use colons (and it doesn't have any Interfere from OS because hard coded string) so of course it fails if i fix it (getMAC function) to use correctly colons style like in readme/examples tries to show user to use in configs. More: So that one knows to handle both styles (that function ONLY has OS input and it handles it correctly but i am not talking about that) which is used only at sending to include MAC.. but getMAC fails as it creates forced hyphen style always and IS used with function parse (so example and readme example of passing colon style MAC to this function is never gonna work as long as i understand because result is the same than hard coded test.. It has wrong style MAC (colons)). ...But i can be wrong and happy to learn when pointed all as i need admit i learned that for loop already and wanna thank of it that you didn't just write ready code :) |
Unnecessary broken file
Unnecessary tests
fix'd
Old typos
Mostly my main point only is to change handling MAC addresses from '-' to ':' so that examples/server.js works with Static IP assignments as should + console.log shows with ':' too now.
Forced order of DHCP Offer message can be left out as it didn't yet fix Microsoft devices problem.