davesteele / comitup

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

expand ap_name suffix options #153

Closed sistemicorp closed 3 years ago

sistemicorp commented 3 years ago

Add this capability,

    # This defines the name used for the AP hotspot, and for the ZeroConf
    # host name. The "<nnn>|<MMMM>|<ssss>" string, if present, will be replaced
    # as follows,  <nnn> instance-unique, persistent number of the same length (up to 4)
    #              <MMMM> device WiFi MAC address of the same length (up to 12)
    #              <ssss> device serial number of the same length (up to 16)
    # Similarly, the string "<hostname>" is replaced with the system hostname.
sistemicorp commented 3 years ago

Note that the implementation does not enforce a length on the AP name <###> spec. I will fix that with next comit.

sistemicorp commented 3 years ago

Funny thing, I didn't realise <hostname> was an option. My boot script changes my hostname to name-MMMM, just like what this change request does thru comitup.conf. Meaning, if I was paying attention, I could have just used <hostname>.

Still, I think this is useful.

sistemicorp commented 3 years ago

Here is a test page to the regex

The new regex allows the id spec to appear anywhere in the apname string.

The logic only allows one id spec. The idea is to create a unique apname, and thus I think only one should be sufficient.

Validation of the apname string has been removed. Rather the logic only looks for the presence of the id spec.

davesteele commented 3 years ago

f7697b8fa7 shows what I want to see from baseline ap_name support.

sistemicorp commented 3 years ago

Understood. I tested many combinations manually, restarting comitup every time.
I haven't looked how to run the unit tests. I can add to the test cases if/when I can figure how to run.

As it is now, if an invalid spec is set in conf, no substitution will occur. And thus an unexpected SSID will happen, for example A<nnnnnnnnn> is invalid, no substitution occurs, and thus that becomes the SSID. On one hand, the developer will be able to see this SSID and get a big hint as to what to check.

Because <,> are valid SSID characters, we can't invalidate based on their presence.

davesteele commented 3 years ago

pytest, nox, tox, and "setup.py test" are all supported. I'd start with nox.

from a python3 virtualenv:

After a successful nox run, just install the pytest deb package and run py.test-3.

sistemicorp commented 3 years ago

nox tests updated, and passing.
Added cases,

    Case("<M>", "1"),
    Case("<MMMMMM>", "654321"),
    Case("<hostname>-<MMMMMM>", "host-654321"),
    Case("<MMMM>-<hostname>", "4321-host"),
    Case("<MMMM><hostname>", "4321host"),
    Case("<s>", "1"),
    Case("<ssssss>", "654321"),
    Case("<hostname>-<ssssss>", "host-654321"),
    Case("<ssss>-<hostname>", "4321-host"),
    Case("<ssss><hostname>", "4321host"),

Passing,

test/test_statemgr.py ....................... 

I didn't do the 2nd part yet.

davesteele commented 3 years ago

Pytests look good.

sistemicorp commented 3 years ago

Sorry, I don't understand: run py.test-3 I did install the pytest deb package and ran the nox tests and now flake8 tests pass...

davesteele commented 3 years ago

"py.test-3" is the executable installed by the python3-pytest deb package.

Looks good. Merging.

davesteele commented 3 years ago

There was a second call to expand_ap() - e7daa090ad