tzapu / WiFiManager

ESP8266 WiFi Connection manager with web captive portal
http://tzapu.com/esp8266-wifi-connection-manager-library-arduino-ide/
MIT License
6.57k stars 1.97k forks source link

Refactor: static ip flags #1051

Open dontsovcmc opened 4 years ago

dontsovcmc commented 4 years ago

Sorry if I don't understand all use cases, but doubling static ip flags '_disableIpFields' '_staShowStaticFields' looks terrible.

There are only 2 cases:

  1. User wants to use static ip address to connect to internet
  2. User wants to use DHCP

I can refactor it, if needed.

P.S. It will be interesting to research statistic of using WifiManager methods.. I think there are only few popular cases. Others are brake the development.

P.P.S. optionalIPFromString also is outdated.

tablatronix commented 4 years ago

It is so you can force showing the fields if you are setting them in code or from elsewhere.

Maybe you can be more specific with what the problem is? Do you mean confusing?

_disableIpFields is special case, it is not exposed with a setter for that reason

Do you have an idea how to clean it up ?

tablatronix commented 4 years ago

P.P.S. optionalIPFromString also is outdated. ?

dontsovcmc commented 4 years ago
  1. i can't understand why somebody hardcode static ip and don't want to show it on the menu

  2. i don't have any problem. i think we should simplify source code.

  3. i don't understand the function of _disableIpFields

  4. optionalIPFromString uses to ESP8266 core 2.1.0 message. it's outdated, so we can remove it and use

    IPAddress ip;
    if (ip.fromString(server->arg(FPSTR(S_ip)))) {
    _sta_static_ip = ip;
    }
  5. also should be changed to IPAddress class functions: isIp() toStringIp()

dontsovcmc commented 4 years ago

@tablatronix offtopic: Why getWiFiSSID()= WiFi_SSID() & getWiFiPass()=WiFi_psk() functions were added? There are the same...

tablatronix commented 4 years ago

They are public helpers, have better names, atm they are just aliases, but if we needed to have different logic for internal wrappers and public ones, we can change them. This is for future abstractions. It also gives us public methods to add docblocks to for auto documentation