Do not include network aliases (identified by same MAC)

[htool]

Starting a listener on a network alias fails, so do not include them in allAddresses function.

[htool]

Solves #355

[coveralls]

Pull Request Test Coverage Report for Build 5175877906

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.1%) to 91.795%

---

Files with Coverage Reduction	New Missed Lines	%
lib/server.ts	2	86.93%

Totals
Change from base Build 5048002328:	-0.1%
Covered Lines:	2860
Relevant Lines:	3073

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 9362935462

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
• 45 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.2%) to 91.676%

---

Files with Coverage Reduction	New Missed Lines	%
lib/outgoing_message.ts	1	97.93%
lib/server.ts	44	86.56%

Totals
Change from base Build 5048002328:	-0.2%
Covered Lines:	2889
Relevant Lines:	3106

---

💛 - Coveralls

[Apollon77]

Additionally to the change request : how this detects network aliases? Is it secured that the main entry always comes before the Alia's in the list? No other option to detect them? Could I see such a list with aliases included please?

[htool]

One of them gets included. No other way to see other than matching macs.

[htool]

Good catch on my copy mistake.

Scenario is when having multiple ips on the same interface.
The interface can only have 1 listener.

[JKRhb]

Can you maybe post a code snippet or script for reproducing the error? I tried adding an alias to a network interface (using something along the lines of ip link set foo alias bar) but the original code seems to still work fine afterward. It is not unlikely that I made an obvious mistake here, though.

[htool]

Ip addr add to add a second ip to an interface. It's not alias but additional ip on same interface.

[Apollon77]

Hm .. but why this do not work? Do we not listen on that up, so multiple listeners should work. Which error exactly you get?

[htool]

Adding second ip to existing wifi interface:
$ sudo ip addr add 192.168.1.125 dev wlp64s0

allAddresses will list both IPs, while being the same interface.

Error: addMembership EADDRINUSE
    at Socket.addMembership (node:dgram:849:11)
    at /home/hans/.signalk/node_modules/coap/dist/lib/server.js:207:34
    at Array.forEach (<anonymous>)
    at Socket.<anonymous> (/home/hans/.signalk/node_modules/coap/dist/lib/server.js:206:58)
    at Socket.onListening (node:dgram:252:7)
    at Socket.emit (node:events:513:28)
    at startListening (node:dgram:181:10)
    at node:dgram:366:7
    at processTicksAndRejections (node:internal/process/task_queues:84:21)
    at runNextTicks (node:internal/process/task_queues:65:3)
    at processTimers (node:internal/timers:499:9) {
  errno: -98,
  code: 'EADDRINUSE',
  syscall: 'addMembership'
}

[JKRhb]

Thank you for the stack trace! I think the problem here is actually that the server tries to join the same multicast group twice. For some reason, it iterates over the list of IP addresses instead of a list of network interfaces. Therefore, if you add another IP address (which appears to be used as an identifier here), calling Socket.addMembership() will fail.

Maybe we could try replacing the addresses as identifiers with os.getNetworkInterfaces() to get the proper interface names..? I need to have a closer look into this later.

[JKRhb]

Since allAddresses is apparently only used for getting the addresses as interface identifiers for the multicast configuration, I think the approach proposed by @htool is actually an appropriate fix. Maybe we could rename the allAddresses method or add some documentation to make its (de-facto) purpose a bit clearer.

[htool]

@Apollon77 What still needs to be done to move this forward? I don't see how I can further influence it.

[Apollon77]

@htool Maybe see the last comment from @JKRhb as reveiw feedback:

Maybe we could rename the allAddresses method or add some documentation to make its (de-facto) purpose a bit clearer.

And rename method or add acomment describing the behavior and why? Then I think we can merge here.

@JKRhb any other opinion?

[htool]

I've added a comment to explain why the repeating MAC check is there.

[JKRhb]

Thank you, @htool, looks good to me :)

Unfortunately, the tests under Node 20 are still failing, but since that is a general problem that is also present on the main branch, I guess we can go forward with merging this PR. Hopefully, we'll be able to find a fix for that soon.