I tested a few configurations. Working ones:
mimxrt (Teensy 4.1)
Pico_W

Non-Working:
Pico_NINAW10 (Arduino nano connect 2040)
PYBV11 + W5500
PYBD-SF6

In all cases the additional network module methods do not exist. For PyBoard + W5500 it is anyhow better to wait until PR #9021 is merged. Then modnwwiznet5k.c and network_wiznet5k.c are dropped.

Once again, thank you @robert-hh for your help in this PR.

I tested a few configurations. Working ones: mimxrt (Teensy 4.1) Pico_W

Good ! I tested for WIZNET_5500_EVB_PICO and WIZNET_5100S_EVB_PICO boards.

Non-Working: Pico_NINAW10 (Arduino nano connect 2040) PYBV11 + W5500 PYBD-SF6
In all cases the additional network module methods do not exist.

That's strange as the .hostname() method is global to the network module. So as soon as the MICROPY_PY_NETWORK constant is defined for a board, it shall be present anyway (except for CC3200 port which seems to use
its own mod_network.c and .h files). Now for a specific nic board, the ping using the hostname + .local is available only if it uses the LWIP library (i.e. the MICROPY_PY_LWIP constant is defined), and the LWIP_MDNS_RESPONDER constant is defined for this board. Same applies for the extra mDNS functions that require on the top of the two previous constants that the LWIP_DNS_SUPPORT_MDNS_QUERIES constant is defined as well.

For PyBoard + W5500 it is anyhow better to wait until PR #9021 is merged. Then modnwwiznet5k.c and network_wiznet5k.c are dropped.

Argh !! When I try to synchronize the branch with the original master from the GitHub web page, it adds a message that does not follow the requested commit message format (ex:Merge branch 'micropython:master' into Hostname-mDNS). How can I avoid this?

The hostname method exists in the RP2040_NINAW10, but not the other ones like mdns_add_service().

bt.w.: It seems strange that one has to configure the nic for dhcp to get the mdns services working. What happens if I want to use fixed IP addresses?

About git: I usually to not merge the master into my branches, but rebase them on top of the current master after I updated the master.
git rebase master

Nevertheless, PR 9021 is not merged yet.

@robert-hh,

The hostname method exists in the RP2040_NINAW10, but not the other ones like mdns_add_service().

That's normal as it does not use LWIP library (which has the mDNS server embedded), or does it?

bt.w.: It seems strange that one has to configure the nic for dhcp to get the mdns services working. What happens if I want to use fixed IP addresses?

Nope. You can use a fixed address as well (I use it personally):

>>> import network
>>> network.hostname("my-host")
>>> nic = network.WIZNET5K()
>>> nic.active(True)
>>> nic.ifconfig(("192.168.1.100", "255.255.255.0", "0.0.0.0", "0.0.0.0"))
>>> svc_token = nic.mdns_add_service("", "_http", network.DNSSD_PROTO_TCP, 80)
>>> nic.mdns_rename_service(svc_token, "my_http_server")
>>> nic.mdns_remove_service(svc_token)

By the way, wouldn't be nice to be able to have default implicit values for the tuple content not to have to enter all the values:
("192.168.1.100",) => ("192.168.1.100", "255.255.255.0", "0.0.0.0", "0.0.0.0")
("192.168.1.100", "255.255.255.128")=> ("192.168.1.100", "255.255.255.128", "0.0.0.0", "0.0.0.0")
Etc.

About git: I usually to not merge the master into my branches, but rebase them on top of the current master after I updated the master. git rebase master

Ok. Thanks. I'll use it manually too.

Nevertheless, PR 9021 is not merged yet.

OK. I'll wait until this is done. Hoping that it won't make any conflict because of the removal of these 2 extra files...

That's normal as it does not use LWIP library (which has the mDNS server embedded), or does it?

No, LWIP is not used.

Nope. You can use a fixed address as well (I use it personally):

Just tried it and it works. I was just confused during my mimxrt test, because there the dhcp request is automatically started. So you do not need a ifconfig() call at all.

By the way, wouldn't be nice to be able to have default implicit values for the tuple content not to have to enter all the values.

But you have to set the default values. And that's only inconvenient in test situations, where you change that all the time. Besides that, it seems easy to write a short python wrapper which you can put in main.py

Hoping that it won't make any conflict because of the removal of these 2 extra files...

That will be a conflict. So better wait and remove the changes to these files from your PR.

Thanks @omogenot & @robert-hh ! I haven't had a chance to review this in detail, but conceptually this seems like a good idea.

Just out of curiosity, rather than putting the mdns_* methods on each of the NIC objects, can these go directly on network too, like hostname? Would you ever want to register an mdns service on just a single interface? I understand that only interfaces that use LWIP can support this, but the way it works right now is that LWIP is all-or-nothing -- it's not possible for example to have e.g. a pico w also using a wiznet with WIZNET5K_WITH_LWIP_STACK set to false, so you know that all your interfaces should support this. However I guess we need a way to make sure it only applies to active interfaces.

This will obviously need to be rebased when #9021 but when you do, can I suggest that because LWIP is all or nothing, the mod_network_nic_type_t struct should just have either the netif ptr or all of the API methods. No need to do this via an extra level of indirection. And then, like you've done in this PR, make all interfaces use mod_network_nic_type_t but just set the appropriate fields. Also please rename netif_ptr to lwip_netif and type it as struct netif not void*.

Also need to mention that of course none of this applies to ESP32/ESP8266... I don't know what the right answer is there. In the short term I guess the hostname and mdns methods methods need to be re-implemented in esp32/modnetwork.c (not this PR though). Longer term I guess we need to find a way to refactor the ESP networking into extmod/modnetwork.c.

Thanks @jimmo for your support and help.

Just out of curiosity, rather than putting the mdns_* methods on each of the NIC objects, can these go directly on network too, like hostname? Would you ever want to register an mdns service on just a single interface? I understand that only interfaces that use LWIP can support this, but the way it works right now is that LWIP is all-or-nothing -- it's not possible for example to have e.g. a pico w also using a wiznet with WIZNET5K_WITH_LWIP_STACK set to false, so you know that all your interfaces should support this. However I guess we need a way to make sure it only applies to active interfaces.

I thought about this too, however yes it's possible to register a service for one nic board only (you can bind a socket to a specific IP). It might not look logical but I have seen so many strange things in my programming life ;-) ... Since this service registration is limited (as of the time of writing to 1 service per nic board only), you may want to optimize your services registrations.

This will obviously need to be rebased when #9021 but when you do, can I suggest that because LWIP is all or nothing, the mod_network_nic_type_t struct should just have either the netif ptr or all of the API methods. No need to do this via an extra level of indirection. And then, like you've done in this PR, make all interfaces use mod_network_nic_type_t but just set the appropriate fields. Also please rename netif_ptr to lwip_netif and type it as struct netif not void*.

I though about this as well, but I cannot make this assumption that it is LWIP or nothing (again strange things could happen in the future...), and therefore I decided to add this indirection to save ROM space and keep mod_network_nic_type_t decoupled from LWIP as much as possible. This indirection costs 1 pointer size of ROM, and saves 14 pointers size of ROM when not used (I am kind of picky on memory footprints).
Regarding the naming convention, no problem I'll do it. However, I suggest to keep the type as void * as for nic boards that do not use the LWIP library, the struct netif is unknown. It would need to declare this type through a lib/lwip/src/include/lwip/netif.h include and doing so could possibly define some conflicting other types.

Once again, these comments are just to explain how I came with these solutions whether it makes sense or not. I will align to everyone's decision, you guys have much more experience in this project than I have :-).

I though about this as well, but I cannot make this assumption that it is LWIP or nothing (again strange things could happen in the future...)

In that case, in the future we end up with a third set of possible fields in the struct.

This indirection costs 1 pointer size of ROM, and saves 14 pointers size of ROM when not used (I am kind of picky on memory footprints).

The point is that when you're compiling with LWIP, the struct only contains the single pointer. The others are #ifdef'ed out.

However, I suggest to keep the type as void * as for nic boards that do not use the LWIP library, the struct netif is unknown.

There is currently no board that has this field that doesnt' have LWIP. As above, in the future if we add another option then we can type the field for that stack as required.

@jimmo
What about preparing the future and do the following in modnetwork.h:
a) create an enum that describes the 'contents' of the pointer.
b) use one (and only one) pointer that contains what is described in the enum above.

typedef enum {
    custom_ip_stack = 0,
    lwip_ip_stack,
} mod_network_ip_stack_type;

typedef struct _mod_network_nic_type_t {
    mp_obj_type_t base;
    mod_network_ip_stack_t ip_stack_type;
    void *ip_stack_info;
} mod_network_nic_type_t;

mod_network_set_hostname function in modnetwork.c would test for ip stack type and become:

const char *mod_network_set_hostname(const char *host) {
    // New hostname can't be NULL nor zero length string.
    if (host && *host) {
        strncpy(hostname, host, MICROPY_PY_HOSTNAME_LENGTH);
    }
    hostname[MICROPY_PY_HOSTNAME_LENGTH] = 0;
    #if LWIP_MDNS_RESPONDER
    for (mp_uint_t i = 0; i < MP_STATE_PORT(mod_network_nic_list).len; i++) {
        mod_network_nic_type_t *nic = (mod_network_nic_type_t *)mp_obj_get_type(MP_STATE_PORT(mod_network_nic_list).items[i]);
        if ((nic->ip_stack_type == lwip_ip_stack) && nic->ip_stack_info) {
            mdns_resp_rename_netif((struct netif *)nic->ip_stack_info, hostname);
        }
    }
    #endif  // LWIP_MDNS_RESPONDER
    return (const char *)hostname;
}

And populate the mod_network_nic_type_t in each nic board depending on the ip stack used (example in network_wiznet5k.c):

#if WIZNET5K_WITH_LWIP_STACK
const mod_network_nic_type_t mod_network_nic_type_wiznet5k = {
    .base = {
        { &mp_type_type },
        .name = MP_QSTR_WIZNET5K,
        .make_new = wiznet5k_make_new,
        .locals_dict = (mp_obj_dict_t *)&wiznet5k_locals_dict,
    },
    .ip_stack_type = lwip_ip_stack,
    .ip_stack_info = (void *)&wiznet5k_obj.netif,
};
#else // WIZNET5K_PROVIDED_STACK
const mod_network_api_func_type_t mod_network_api_func_wiznet5k = {
    .gethostbyname = wiznet5k_gethostbyname,
    .socket = wiznet5k_socket_socket,
    .close = wiznet5k_socket_close,
    .bind = wiznet5k_socket_bind,
    .listen = wiznet5k_socket_listen,
    .accept = wiznet5k_socket_accept,
    .connect = wiznet5k_socket_connect,
    .send = wiznet5k_socket_send,
    .recv = wiznet5k_socket_recv,
    .sendto = wiznet5k_socket_sendto,
    .recvfrom = wiznet5k_socket_recvfrom,
    .setsockopt = wiznet5k_socket_setsockopt,
    .settimeout = wiznet5k_socket_settimeout,
    .ioctl = wiznet5k_socket_ioctl,
};
const mod_network_nic_type_t mod_network_nic_type_wiznet5k = {
    .base = {
        { &mp_type_type },
        .name = MP_QSTR_WIZNET5K,
        .make_new = wiznet5k_make_new,
        .locals_dict = (mp_obj_dict_t *)&wiznet5k_locals_dict,
    },
    .ip_stack_type = custom_ip_stack,
    .ip_stack_info = (void *)&mod_network_api_func_wiznet5k,
};
#endif

I keep my indirection to the network api custom functions as this would still keep mod_network_nic_type_t completely independent of any ip stack type for the future (i.e. no #if MICROPY_PY_LWIP reference in the .h file)

@jimmo

Also need to mention that of course none of this applies to ESP32/ESP8266... I don't know what the right answer is there. In the short term I guess the hostname and mdns methods methods need to be re-implemented in esp32/modnetwork.c (not this PR though). Longer term I guess we need to find a way to refactor the ESP networking into extmod/modnetwork.c.

I happen to have an ESP32 based board that works using the GENERIC board definition, although it does not have LAN interface. Do you want me to start a new PR to clean up a bit this code:
a) there are specific modnetwork.c and modnetwork.h files that shall probably renamed as esp32network for esp32 specific functions and use common extmod/modnetwork files for common functions instead.
b) everything is tossed to the network object, whereas constants definitions shall rather be defined by interface type (for example STA_IF shall be related to WLAN object only)
Etc.
Who else shall be included in this work to validate my choices/strategies/variable and function names, etc.

I thought about this too, however yes it's possible to register a service for one nic board only (you can bind a socket to a specific IP). It might not look logical but I have seen so many strange things in my programming life ;-) ... Since this service registration is limited (as of the time of writing to 1 service per nic board only), you may want to optimize your services registrations.

I'm not quite sure I follow. Does this mean that mdns_add_service can only be called once on an interface? Where does this limitation come from (LWIP?).

Just because it could be possible to support having unique services per NIC, doesn't mean we should. If we can reduce code size and complexity it might be worth just making them global.

What about preparing the future and do the following in modnetwork.h:
a) create an enum that describes the 'contents' of the pointer.
b) use one (and only one) pointer that contains what is described in the enum above.

@omogenot I get what you're saying but there are many other parts that currently work on the assumption that you're either compiling with LWIP or custom stack, and never both. For example, whether you get modsocket.c or modlwip.c -- they both provide the socket module, you cannot have both compiled at the same time. We may decide to address this one day but let's worry about that when we get there, perhaps with something a bit like what you're describing. It's probably more likely that we would add a third (mutually-exclusive) IP stack before supporting multiple concurrent stacks.

The main point though is that your proposal adds two words per NIC type (the enum value and the pointer) which currently don't confer any benefit. If it's an LWIP build, then the enum value will always be LWIP, etc.

Do you want me to start a new PR to clean up a bit this code:

I agree it would be good to do this (and along the way I'm definitely keen to see some standardisation between ESP32 and the other ports with things like the constants on the network module, e.g. AUTH_*, etc). We also should be able to combine esp32/modsocket.c into extmod/modlwip.c.

But let's figure out what we want to do with the API first in this PR -- i.e. whether adding network.hostname() is the right thing to do and where to put the mdns_* functions.

@jimmo

I'm not quite sure I follow. Does this mean that mdns_add_service can only be called once on an interface? Where does this limitation come from (LWIP?).

Yes. In lib/lwip/doc/mdns.txt we can read:

The max number of services supported per netif is defined by MDNS_MAX_SERVICES,
default is 1.

Just because it could be possible to support having unique services per NIC, doesn't mean we should. If we can reduce code size and complexity it might be worth just making them global.

The code complexity is kind of 'simplified' (if I can say so) by the fact that the main part of the process handling the mdns_* functions is written in modnetwork.c as helping functions that are shared by each nic board (just calling it).
I insist (for the last time) on keeping this at the nic board level. This is especially true for the ESP32, for instance, that can have more than 1 nic board (Cable, WiFi and PPP) active at the same time. You may not want to publish a service (http for instance) on one of the nic, keeping your web server access to a local specific network (direct cable connection) for device configuration and not to other stations on the LAN. But you decide...

@omogenot I get what you're saying but there are many other parts that currently work on the assumption that you're either compiling with LWIP or custom stack, and never both. For example, whether you get modsocket.c or modlwip.c -- they both provide the socket module, you cannot have both compiled at the same time. We may decide to address this one day but let's worry about that when we get there, perhaps with something a bit like what you're describing. It's probably more likely that we would add a third (mutually-exclusive) IP stack before supporting multiple concurrent stacks.

As you mentioned, it's an assumption. Too bad since we opened the hood and started to touch the engine ;-).

The main point though is that your proposal adds two words per NIC type (the enum value and the pointer) which currently don't confer any benefit. If it's an LWIP build, then the enum value will always be LWIP, etc.

Fine by me, the goal was to decouple the mod network.h from any specific network library type to avoid any definition conflict in the future when including this file.

Do you want me to start a new PR to clean up a bit this code:

I agree it would be good to do this (and along the way I'm definitely keen to see some standardisation between ESP32 and the other ports with things like the constants on the network module, e.g. AUTH_*, etc). We also should be able to combine esp32/modsocket.c into extmod/modlwip.c.

But let's figure out what we want to do with the API first in this PR -- i.e. whether adding network.hostname() is the right thing to do and where to put the mdns_* functions.

You're the boss, boss :-). The idea was to open a PR for discussion, I don't know how you proceed in the community to decide for such enhancements. I was just proposing my help because I am still on vacation until the end of the week, after that I'll be back to work and have much less spare time.

I'll wait for you final decisions to modify the source code accordingly or drop this PR (or may be you can modify code directly on my GitHub fork).

Please note that the unix port / settrace_stackless (pull_request) test fails suddenly, but I did not touch anything there.

@jimmo
By the way, the modnetwork.c file contains a function mod_network_find_nic that does not seem right or finished:

mp_obj_t mod_network_find_nic(const uint8_t *ip) {
    // find a NIC that is suited to given IP address
    for (mp_uint_t i = 0; i < MP_STATE_PORT(mod_network_nic_list).len; i++) {
        mp_obj_t nic = MP_STATE_PORT(mod_network_nic_list).items[i];
        // TODO check IP suitability here
        // mod_network_nic_type_t *nic_type = (mod_network_nic_type_t*)mp_obj_get_type(nic);
        return nic;
    }

    mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("no available NIC"));
}

Do you know what it was intended to do? Shall we fix it?

@jimmo
Second thought about the global mdns_* functions. It seems to me more difficult to implement as a global function call, like for hostname, because:

1. The mdns_add_service returns a token value that must be used in the other mdns_* functions. If me make this call global across all nic boards registered to the global network object, we would have to assume that all calls will return the same token value or keep an array of these tokens.
2. When you create a new nic board object, it will be registered to the network object. The latter will call mdns_resp_add_netif for that nic, but how would it remember any mdns_add_service that occurred so far for the other nic boards? Especially if there was more than one call to this function (if the MDNS_MAX_SERVICES constant is greater than 1).
3. This would imply that you shall call the mdns_add_service method after all nic boards have been defined otherwise a weird situation could occur due to the point 2. above if you call mdns_remove_service for a nic board that was not registered initially.

@jimmo @dpgeorge
I'm sorry, I really don't know how this happened ! I definitely did not want to do that... I was just try to update my fork with the current micropython:master, not the opposite.

Edit: OK. Looks like no damage has been done to the repository.
Since we have still a doubt regarding the mdns_* functions implementation, I will split it in two different PRs.

Hey, So i flashed "rp2-pico-w-20220906-unstable-v1.19.1-375-ge90b85cc9.uf2" which is Sept 06th build on Pico_W and network.hostname() is still not an attribute.

>>> import network
>>> network.hostname()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute 'hostname'
>>> 

Appreciate your help.

Thank You!

@maxacode
Hi,
This is normal (or kind of 😄). This PR has been merged with no commits added to the main branch and closed, following a mis-use of GitHub. This PR has been abandoned and replaced by #9086.
Therefore, you will have to wait for this PR #9086 to be merged in the main brach to see this functionality being part of a nightly build or an official release.

@maxacode Hi, This is normal (or kind of 😄). This PR has been merged with no commits added to the main branch and closed, following a mis-use of GitHub. This PR has been abandoned and replaced by #9086. Therefore, you will have to wait for this PR #9086 to be merged in the main brach to see this functionality being part of a nightly build or an official release.

Ahh makes sense, explains it well.

So until then I can just compile this branch to flash it on the Pico, since only this branch has the merged code?

Thanks

@maxacode
You can compile the #9086 branch to get the hostname function. The one linked to this PR has been more or less mixed-up due to some mis-use of GitHub (sorry).

@maxacode You can compile the #9086 branch to get the hostname function. The one linked to this PR has been more or less mixed-up due to some mis-use of GitHub (sorry).

Yes sorry that is what I meant by "this branch" the new one, thats not messed up.

Thank You so much. I am just getting into Micro Controllers and its a whole new world. I know Python for years but this is new for me.