micropython / micropython-esp32

Old port of MicroPython to the ESP32 -- new port is at https://github.com/micropython/micropython
MIT License
673 stars 216 forks source link

ESP32: Manual setting of DNS address with ifconfig() fails #210

Closed robert-hh closed 6 years ago

robert-hh commented 6 years ago

An attempt to set manual ip parameters with ifconfig() does not accept the DNS parameter. Instead it is set to 0.0.0.0 Example:

>>> wlan.ifconfig(('10.0.0.163', '255.255.255.0', '10.0.0.240', '192.76.144.66'))
I (109307) event: sta ip: 10.0.0.163, mask: 255.255.255.0, gw: 10.0.0.240
I (109307) wifi: GOT_IP
>>> wlan.ifconfig()
('10.0.0.163', '255.255.255.0', '10.0.0.240', '0.0.0.0')
robert-hh commented 6 years ago

Adding the line: dns_setserver(0, &dns_addr); after line 364 in modnetwork.c works as a fix.

MrSurly commented 6 years ago

@robert-hh Perhaps submit a PR? Edit: I can do this if you'd rather; doesn't appear you've forked this repo.

robert-hh commented 6 years ago

Thanks. Yes, and I'm a little bit shy to fork it just for that line. The section should then look like:

        if (self->if_id == WIFI_IF_STA) {
            esp_err_t e = tcpip_adapter_dhcpc_stop(WIFI_IF_STA);
            if (e != ESP_OK && e != ESP_ERR_TCPIP_ADAPTER_DHCP_ALREADY_STOPPED) _esp_exceptions(e);
            ESP_EXCEPTIONS(tcpip_adapter_set_ip_info(WIFI_IF_STA, &info));
            dns_setserver(0, &dns_addr);

But it's still strange. If I include my interface setup script in the frozen bytecode, setting a static IP address, most of the time getaddrinfo() still fails. If I run the same script as python source, it works! Maybe a timing side effect.

MrSurly commented 6 years ago

I implemented your change, but it only seems to work cosmetically. There might be a problem in the IDF.

Edit: It's more complex/subtle -- sometimes it works, sometimes it doesn't. It's weird.

After setting the DNS with ifconfig(info), then doing ifconfig() you see the new DNS, but it can't actually resolve. Internally, I think it caches results, which is why the example below checks different URLs.

Even setting the DNS like this:

x = w.ifconfig()
w.ifconfig(x)

causes DNS lookups to fail.

Test script:

import network as n, socket as s
w = n.WLAN()
w.active(True)
w.connect('Mango', '<PASSWORD>')

s.getaddrinfo('yahoo.com',80)

z = list(w.ifconfig())

s.getaddrinfo('google.com',80)

z[-1] = '8.8.8.8'

w.ifconfig(z)

s.getaddrinfo('reddit.com', 80)
robert-hh commented 6 years ago

That's what I noticed: the same DNS setting works, when part of a python source script either from the FAT file system of frozen scripts, and fails most of the time, when its is bytcode either in a compiled script from the file system or inside frozen bytecode. And it is important that the DNS is set before connecting to an AP. At least, w/o that code change it fails allways.

robert-hh commented 6 years ago

So here is an indication: Making the variable dns_addr in esp_ifconfig() static makes a difference. Then also compiled code would set the DNS address right. I wonder if it is necessary to split that for _get and _set. It does not to seem to be needed. Edit: looking at the code of dns_setserver(), this behaviour is strange, since the data is copied:

void
dns_setserver(u8_t numdns, const ip_addr_t *dnsserver)
{
  if (numdns < DNS_MAX_SERVERS) {
    if (dnsserver != NULL) {
      dns_servers[numdns] = (*dnsserver);
    } else {
      dns_servers[numdns] = *IP_ADDR_ANY;
    }
  }
}
MrSurly commented 6 years ago

Edit: looking at the code of dns_setserver(), this behaviour is strange, since the data is copied:

Yeah, was going to make the same comment. That is weird.

MrSurly commented 6 years ago

Making the variable dns_addr in esp_ifconfig() static makes a difference. Then also compiled code would set the DNS address right ...Edit: looking at the code of dns_setserver(), this behaviour is strange, since the data is copied:

Okay, I figured this out. When you make dns_addr static, it initializes the whole structure to zero. That includes the type field, which is (coincidentally) supposed to be 0 for IPV4. For non-static, the type field will be uninitialized (e.g. effectively random).

PR incoming soon.

MrSurly commented 6 years ago

PR: https://github.com/micropython/micropython-esp32/pull/213

dpgeorge commented 6 years ago

Should now be fixed by the above PR.