davesteele / comitup

Bootstrap Wifi support over Wifi
https://davesteele.github.io/comitup/
GNU General Public License v2.0
320 stars 54 forks source link

initial for eth0 support #159

Closed sistemicorp closed 3 years ago

sistemicorp commented 3 years ago

Working prototype to support eth0 as a connection option.

sistemicorp commented 3 years ago

This is "working" for me now. That is, eth0 is now a connection option. The external callback will get "CONNECTED" when wired is connected. Plug'n'Play between eth0 and wlan0 seems to be working.

The threading.Timer is a hack to resolve the eth0 state at boot up.

I spent a few hours today trying to make this work following the send_cb(nm_dev_connect) convention. But I failed again.

davesteele commented 3 years ago

A quick looks suggests this could work OK. Before merging, I would need to see a way for this to be optional via a config variable.

sistemicorp commented 3 years ago

On boot up, I am seeing inconsistent result that the wired CONNECTED state is not being posted to the external callback.

The first log from case of not working. My callback log lines are cmd: ... (which I manually inserted here for timing),

    Starting Tue Apr  6 13:18:28 EDT 2021
    2021-04-06 13:18:40,472 - comitup - INFO - Starting comitup
    2021-04-06 13:18:40,633 - comitup - INFO - using SSID: p1125-man-asy
    2021-04-06 13:18:40,848 - comitup - INFO - Setting state to HOTSPOT
    2021-04-06 13:18:40,864 - comitup - INFO - Activating hotspot
    cmd: HOTSPOT Tue Apr  6 13:18:41 EDT 2021 ----------------------------------
    2021-04-06 13:18:45,741 - comitup - INFO - nmm - wired CONNECTED
    2021-04-06 13:18:47,263 - comitup - INFO - Setting state to CONNECTED
    // CONNECTED state never gets posted

This is a second boot, and working,

    2021-04-06 13:27:43,171 - comitup - INFO - Starting comitup
    2021-04-06 13:27:43,321 - comitup - INFO - using SSID: p1125-man-asy
    2021-04-06 13:27:43,547 - comitup - INFO - Setting state to HOTSPOT
    2021-04-06 13:27:43,565 - comitup - INFO - Activating hotspot
    cmd: HOTSPOT Tue Apr  6 13:27:44 EDT 2021 ----------------------------------
    2021-04-06 13:27:48,426 - comitup - INFO - nmm - wired CONNECTED
    2021-04-06 13:27:49,864 - comitup - INFO - Setting state to CONNECTED
    cmd: CONNECTED Tue Apr  6 13:27:50 EDT 2021 ----------------------------------

Because of the log line INFO - nmm - wired CONNECTED I believe set_state('CONNECTED') is being called,

if state in [NetworkManager.NM_DEVICE_STATE_ACTIVATED, ]:
    wired_is_connected = True
    log.info("nmm - wired CONNECTED")
    set_state('CONNECTED')
elif wired_is_connected:
    wired_is_connected = False
    set_state('HOTSPOT')
    log.debug("nmm - wired {}".format(state))
else:
    log.debug("nmm - wired state {}".format(state))

In the timing, you can see the 5.0 sec delay of the timer posting the wired state... I can see if increasing that timer helps. But my app sees the HOTSPOT post almost right after activating hotspot, so I don't see a race condition with 5.0 seconds...

sistemicorp commented 3 years ago

This last change seemed to have fixed eth0 CONNECTED state transition. Before this change, many times the eth0 state change was not propagated to the external callback, although the state was queued. I think because the wifi device was "enabled", its state trumped the eth0 state.

Q: Does the 4fb5c95 change make sense? (apologise upfront for not be able to follow the callback/queuing system)

I will perform more tests...

davesteele commented 3 years ago

Q: Does the 4fb5c95 change make sense?

I don't think so. It can fail if the Ethernet changes state after the enable call.

Maybe if the check were added as a wrapper around the _fn's, or the functions themselves did the wired check.

sistemicorp commented 3 years ago

If Ethernet changes state after the enable call, doesn't wired_changed_state() get called and then set_state('HOTSPOT') would be called and then the "normal" AP system is back in place...

def wired_changed_state(state, *args):
    global wired_is_connected

    from comitup.states import set_state

    if state in [NetworkManager.NM_DEVICE_STATE_ACTIVATED, ]:
        wired_is_connected = True
        log.info("nmm - wired CONNECTED")
        set_state('CONNECTED')
    elif wired_is_connected:
        wired_is_connected = False
        set_state('HOTSPOT')

My "graceful" testing shows that eth0 states are being picked up and comitup AP states are as expected. But I am not yet trying to be mean and find race conditions...

davesteele commented 3 years ago

It's the race conditions that are mean.

davesteele commented 3 years ago

I haven't looked at your latest in detail, but I have taken a look at how I might go about it:

sistemicorp commented 3 years ago

Thank you Dave. Those hints may help me. I spent the last few days trying to get things to work, and I tried using more of the states and architecture that WiFi was using.... but I failed... I have a commit of it, full of logging, but I didn't push it.

Today I backtracked and trying your original suggestion of managing eth0 externally and start/stop comitup service. I think I am making good headway on that... know in a few hours or tomorrow.

This pull request should probably be closed.

davesteele commented 3 years ago

I appreciate the effort.