search

adafruit / nina-fw

Firmware for u-blox NINA W102 WiFi/BT module
85 stars 43 forks source link

Setting a custom hostname does not work #35

Closed Gadgetoid closed 3 years ago

Gadgetoid commented 3 years ago

The upstream WiFiClass::hostname copies the incoming "name" value into a "_hostname" char array here:

https://github.com/arduino/nina-fw/blob/9b4c4355cb82c6ebbdbf9bc178696e3b60712026/arduino/libraries/WiFi/src/WiFi.cpp#L420-L423

And then re-uses it on a system event to update the hostname, here:

https://github.com/arduino/nina-fw/blob/9b4c4355cb82c6ebbdbf9bc178696e3b60712026/arduino/libraries/WiFi/src/WiFi.cpp#L712

By contrast, this fork attempts to update the value immediately like so:

https://github.com/adafruit/nina-fw/blob/ec9e20f5085a121bfeb6348a9ab98bba69193369/arduino/libraries/WiFi/src/WiFi.cpp#L321-L324

And then always uses the default hostname in handleSystemEvent:

https://github.com/adafruit/nina-fw/blob/ec9e20f5085a121bfeb6348a9ab98bba69193369/arduino/libraries/WiFi/src/WiFi.cpp#L607

The practical implications of this seem to be that it's not possible to set the hostname, since I have tried issuing the command to do this and received a response.

The fix for this originates from this PR - https://github.com/arduino/nina-fw/pull/24 - which doesn't seem to have made it into this fork.

ladyada commented 3 years ago

thanks, please make a PR for it!

Gadgetoid commented 3 years ago

thanks, please make a PR for it!

I was worried you might say this.

Already half way down the rabbit hole of figuring out how to compile and test :laughing:

brentru commented 3 years ago

@Gadgetoid

The practical implications of this seem to be that it's not possible to set the hostname,

Why would someone want to do this - what's the purpose in the context of this lib.? (asking b/c I'm trying to understand the practical consideration for the issue).

Gadgetoid commented 3 years ago

Well twofold:

  1. It’s an implemented feature that just doesn’t work as expected- so the principle of least surprise would suggest it should (and since it can work… might as well fix it!)
  2. It’s useful - for beginners who aren’t used to scanning their network or perusing their router configuration - to set a known hostname so they can access their device intuitively

It was 2 that sent me down the rabbit hole chasing this bug, since a customer had tried to set a hostname to no avail and was - as per 1 - understandably confused 🤣