Port from eui48 to macaddr

[milliams]

eui48 has a dependency on the deprecated rustc-serialize package so convert to use macaddr.

Fixes #140

[dtantsur]

Every time I leave this project for a couple of month, the first thing I need to do on return is to fix the CI jobs... Here we go again.

[dtantsur]

@milliams could you rebase this patch on master? Otherwise, I don't know how to make Github pick up the updated CI configuration.

[milliams]

@milliams could you rebase this patch on master? Otherwise, I don't know how to make Github pick up the updated CI configuration.

Rebased

[dtantsur]

Seems to be an actual failure caused by this patch, see build logs: Cannot find the port attached to the server: Error { kind: ProtocolError, message: \"error decoding response body: invalid type: string \\\"fa:16:3e:69:20:e3\\\", expected an array of length 6 at line 1 column 197\", status: None }

[milliams]

It seems there's some difference in how this package serialises its data. I'll have a look into it.

[milliams]

As far as I can tell, macaddr only supports serde for its [u8; 6] format so it cannot automatically parse things like "fa:16:3e:3f:a7:58". I'll carry on poking but it's possible this PR will be dropped. Unfortunately, neither macaddr nor eui48 seem to be actively maintained so fixing either issue may take some time.

[dtantsur]

@milliams ouch. Thanks for the investigation. Worst case, we'll need to introduce our own type with the bare minimum features.

[dtantsur]

@milliams I've looked a bit into this. While macaddr is not perfect, it does have a FromStr implementation https://docs.rs/macaddr/latest/macaddr/struct.MacAddr6.html#impl-FromStr, which means we can have a custom deserialization function. WDYT?

[milliams]

That could work. I'll have a quick look at it. I've learned a lot more about Serde since I last looked at this, so I might have a chance :) Certainly the FromStr seems to be able to parse the hex-format correctly.

[milliams]

For now I've gone the route of implementing a new type which wraps macaddr::MacAddr6. The issue I found with using custom serde with functions is that it circumvents some of the default logic in serde and so we'd have to implement custom functions for both of MacAddr6 and Option<MacAddr6>, as well as keep track of which ones need serde's default flag and implement our own skip_serialization_if logic.

I'm happy to go with another method if you think it's better API-wise.

[dtantsur]

The CI fails because of missing Display (which I think should be the same as to_string())

[milliams]

I will add impl Display.

[dtantsur]

I'm curious why you need this if you have Deref

[milliams]

It seems to be needed as the serde line:

...
 #[serde(skip_serializing_if = "MacAddress::is_nil")]
pub mac_address: MacAddress,
...

specifies a path to a function, so if I remove the fn is_nil, then it gives the error:

error[E0599]: no function or associated item named `is_nil` found for struct `MacAddress` in the current scope
   --> src/network/protocol.rs:461:35
    |
333 | pub struct MacAddress(macaddr::MacAddr6);
    | --------------------- function or associated item `is_nil` not found for this struct
...
461 |     #[serde(skip_serializing_if = "MacAddress::is_nil")]
    |                                   ^^^^^^^^^^^^^^^^^^^^ function or associated item not found in `MacAddress`

The alternative seems to be, annotating the serde field with:

...
 #[serde(skip_serializing_if = "macaddr::MacAddr6::is_nil")]
pub mac_address: MacAddress,
...

but I thought avoiding having to leak the underlying type directly was worth it.

[dtantsur]

This should have been self.0.fmt(f), I think, but this works for me