Use MacAddr6 type for BSSID.

[reitermarkus]

This is a bit more expressive than [u8; 6] and provides a nice Display and Debug implementation.

[svenstaro]

I think this makes sense and I'd like to see it merged.

[ivmarkov]

Yeah. It is introducing a dependency on a not so popular crate, so to say. And this would be in the very core of embedded-svc, we are not talking embedded_svc::utils::* here.

I'm still contemplating whether we should have a hard-dependency on heapless, let alone on less popular crates.

[svenstaro]

I certainly understand the problem but it seems to be very properly implemented and test, even using const fn where possible and I don't see any heap allocations.

[ivmarkov]

I didn't say it is not properly implemented or not unit tested. It is just not so widely used. Would this indicate that the problem is not so major actually, and we can live with it?

[ivmarkov]

I mean, if you want it, just use it outside of embedded-svc without introducing it as a dependency?

[svenstaro]

That wouldn't be a problem at all! I think this is rather a question of where on the line of convenience vs simplicity embedded-svc is trying to position itself. That's probably your call alone. :)

[ivmarkov]

I'm moving towards simplicity and less convenience actually (e.g. see #16); and even some performance sacrifices. And little to no hard dependencies on external crates for the embedded-svc core (everything not in utils). I.e. eradicate Cow from all traits and structures used by the traits so that they are not depending on alloc at all.

With that said, it (should not) be my sole decision. @jessebraham @bjoernQ @MabezDev

[ivmarkov]

But yeah, I'll likely have to introduce a hard dependency on heapless in order to fix this. Or introduce lifetimes.

[reitermarkus]

FWIW, I would like to use actual meaningful types when possible rather than raw byte arrays.

I'll likely have to introduce a hard dependency on heapless in order to fix this.

It could be implemented as a custom type, i.e. Password { bytes: [u8; 64], len: usize }, also ideally with a Debug implementation that masks the actual password by default.

[reitermarkus]

I implemented this in my own version before this crate existed: https://github.com/reitermarkus/esp32-hello/blob/4babc77e0a91ff38440b26821624d010bd609285/esp-idf-hal/src/wifi/password.rs

[ivmarkov]

Yeah. But then you need to do the same for the ssid field, so no - your type is not going to be named Password, but... MyOwnNoAllocString. Or something. And you'll have to make then '64' a const generic by the way (if not for anything else because 64 just hurts my eyes), so what you'll get is ... the crux of the heapless::String type, aren't you? And then the question becomes, why are you reinventing the wheel? :)

[reitermarkus]

And then the question becomes, why are you reinventing the wheel?

You are suggesting that having dependencies is a bad thing, so I assumed you wanted to reinvent the wheel rather than add a dependency. Of course you can just use heapless::String<64>.

[ivmarkov]

And then the question becomes, why are you reinventing the wheel?

You are suggesting that having dependencies is a bad thing, so I assumed you wanted to reinvent the wheel rather than add a dependency. Of course you can just use heapless::String<64>.

And then the question becomes, why are you reinventing the wheel?

You are suggesting that having dependencies is a bad thing, so I assumed you wanted to reinvent the wheel rather than add a dependency. Of course you can just use heapless::String<64>.

Are you misinterpreting my words? I was suggesting that depending on a crate which has 1 star, for a functionality that is not so important (how many people are actually setting the bssid in their code and how many of those would object that this is a byte array?) is probably not a great idea. Now, asking people to use a ([u8; 64], usize) pair, where they could use something easily castable from/to &str, or implementing what is essentially a heapless::String equivalent might be going too far, so yeah, for that case I think it might be OK to depend on heapless, which is much more recognized in the embedded world.

[reitermarkus]

Are you misinterpreting my words?

I guess I did, sorry.

depending on a crate which has 1 star

Sure, it may have 1 star on GitHub, but it has 500k downloads on crates.io which I think is more indicative of its usage in the Rust ecosystem.

[ivmarkov]

Sure, it may have 1 star on GitHub, but it has 500k downloads on crates.io which I think is more indicative of its usage in the Rust ecosystem.

That might be a good point actually.

[ivmarkov]

But then Dependents is a bit weak tho.

[ivmarkov]

OK, let's see what the other folks on the esp-rs team have to say. I think we got too much into the weeds here discussing this detail (sorry for naming the PR like that, and please no offence!). It is just that there are so many other important things to do where PRs would be greatly appreciated.

For one, pure Rust async crates for mqtt, http server/client, so that we can get the bare-metal story off the ground, not just the esp-idf-* one!

[bjoernQ]

IMHO in a crate like this (which defines traits to be implemented elsewhere - possibly on bare-metal) we should really care about dependencies (not just dependencies on external crates but even on things like an allocator and atomics)

If it's just about type-safety what about defining a wrapper type for [u8; 6] in this crate? That's zero-cost