NetworkConfiguration / dhcpcd-ui

GTK+ / Qt / Curses interface to dhcpcd
https://roy.marples.name/projects/dhcpcd-ui
21 stars 7 forks source link

RFC: Disconnect when same SSID selected #1

Closed ghost closed 3 years ago

ghost commented 5 years ago

These changes would make it so that when the currently associated entry in the graphical SSID list is selected, that interface is disconnected. This is intended to provide a continuity with the "checkbox" interface paradigm, and has the effect of providing the user with a convenient graphical way to disconnect wireless.

In modifying the "dhcpcd_get_if" function, I made the "type" argument signed in order to introduce a feature whereby the connection could be checked for an interface regardless of type.

I apologize if I failed to consider any design feature, and I express my gratitude for your taking the time to review.

rsmarples commented 5 years ago

There is no need to change the signedness of the type parameter. You could declare DHT_ALL as MAX_UINT (which oddly enough also equals a signed -1). Saying that, there is no need for this really. You get DHT_LINK for all interfaces, and WPA association is done using this type. Other types are for transacations across the link such as DHCP, DHCP6 and RA.

So in your patch you would use:

i = dhcpcd_get_if(wpa->con, wpa->ifname, DHT_LINK);
ghost commented 5 years ago

Thank you. In addition to reverting to an unsigned type, I moved the conditional disconnection to the gui code, as the appearance of the authentication window depends on that routine.

I apologize for the fact I was unable to comprehend why there was a conditional in the header file based on whether or not it was included in libdhcpcd.c, and I do hope I did not mess anything up organizationally. The ifname and con fields in the DHCPCD_WPA struct were needed to implement a proper call to dhcpcd_get_if().

rsmarples commented 5 years ago

This is looking better now. Can you explain why you need the changed to libdhcpcd/dhcpcd.h ?

ghost commented 5 years ago

In the call(s): dhcpcd_get_if(wpa->con, wpa->ifname, DHT_LINK)

gcc returns an error message saying the elements wpa->con and wpa->ifname of the struct do not exist. This is because the original ifdef makes them pointers to void.

Sorry for the late reply.

rsmarples commented 5 years ago

Why can't you use the function dhcpcd_wpa_if(); ?

ghost commented 5 years ago

When attempting to access the members of the struct DHCPCD_WPA designated by wpa->con and wpa->ifname, gcc returns an error with the original header file, saying that wpa is not a struct, therefore it cannot have members. This is because the original header file uses an #ifdef IN_LIBDHCPCD to define DHCPCD_WPA as a struct if and only if the macro IN_LIBDHCPCD is defined. Otherwise, it uses an #else preprocessor directive to define DHCPCD_WPA as a void type. This is illustrated by the following segment taken from the header file:

#ifdef IN_LIBDHCPCD
typedef struct dhcpcd_if {
    struct dhcpcd_if *next;
    const char *ifname;
    unsigned int type;
    unsigned int state;
    const char *reason;

    unsigned int ifflags;
    bool up;
    bool wireless;
    const char *ssid;
    int freq;

    char *data;
    size_t data_len;

    char *last_message;

    struct dhcpcd_connection *con;
} DHCPCD_IF;
#else
typedef struct dhcpcd_if {
    struct dhcpcd_if *next;
    const char *ifname;
    unsigned int type;
    unsigned int state;
    const char *reason;

    unsigned int ifflags;
    bool up;
    bool wireless;
    const char *ssid;
    int freq;
} DHCPCD_IF;
#endif

typedef struct dhcpcd_config {
    struct dhcpcd_config *next;
    char *option;
    char *value;
} DHCPCD_OPTION;

#ifdef IN_LIBDHCPCD
typedef struct dhcpcd_wi_hist {
    struct dhcpcd_wi_hist *next;
    char ifname[IF_NAMESIZE];
    char bssid[IF_BSSIDSIZE];
    int quality;
    int noise;
    int level;
    int strength;
} DHCPCD_WI_HIST;

typedef struct dhcpcd_wpa {
    struct dhcpcd_wpa *next;
    char ifname[IF_NAMESIZE];
    unsigned int status;
    int command_fd;
    char *command_path;
    int listen_fd;
    char *listen_path;
    bool attached;
    struct dhcpcd_connection *con;
} DHCPCD_WPA;

typedef struct dhcpcd_connection {
    struct dhcpcd_connection *next;
    bool open;
    bool privileged;
    int command_fd;
    int listen_fd;
    const char *progname;

    DHCPCD_IF *interfaces;
    DHCPCD_WPA *wpa;
    DHCPCD_WI_HIST *wi_history;

    void (*if_cb)(DHCPCD_IF *, void *);
    void *if_context;
    void (*status_cb)(struct dhcpcd_connection *,
        unsigned int, const char *, void *);
    void *status_context;
    bool wpa_started;
    void (*wi_scanresults_cb)(DHCPCD_WPA *, void *);
    void *wi_scanresults_context;
    void (*wpa_status_cb)(DHCPCD_WPA *, unsigned int, const char *, void *);
    void *wpa_status_context;

    char *buf;
    size_t buflen;

    char *version;
    bool terminate_commands;
    char *error;
    int err;
    int errors;
    unsigned int status;
    bool af_waiting;

    char *cffile;
} DHCPCD_CONNECTION;

#else
typedef void *DHCPCD_CONFIG;
typedef void *DHCPCD_WPA;
typedef void *DHCPCD_CONNECTION;
#endif

I am sorry if I failed to be clear earlier, or if I have left anything out. If there is anything I can do in the future to aid understanding, please let me know. And again, thank you for your time.

rsmarples commented 5 years ago

I understand just fine. Here is the function: https://github.com/rsmarples/dhcpcd-ui/blob/master/src/libdhcpcd/wpa.c#L973 Here is some sample code for src/dhcpcd-gtk/wpa.c:

wpa_configure(DHCPCD_WPA *wpa, DHCPCD_WI_SCAN *scan)
{
    DHCPCD_IF *i = dhcpcd_wpa_if(wpa);
}

Basically, this has been designed such that all the structs are opaque objects outside of libdhcpcd which means I can chop and change it as much as I like without affecting any users of it. This is a very desirable thing to have.

ghost commented 5 years ago

Ah, I did not see that function. It should now be fixed. Thank you.

ghost commented 5 years ago

Is there anything else I forgot? Thank you for your patience.

rsmarples commented 5 years ago

This looks good now. Sadly I don't actually have a X11 dev env right now with wireless, but aim to bring one up shortly after the new year. I'll give it a go then and if it all work fine I'll merge in.

Thanks for working on this!

rsmarples commented 3 years ago

Wow! Sorry for forgetting about this .....

Thanks, merged.