OpenBikeSensorFirmware icon indicating copy to clipboard operation
OpenBikeSensorFirmware copied to clipboard

Change byte order of base MAC in the SoftAP SSID.

Open SubOptimal opened this issue 3 years ago • 8 comments

With this PR the computed SSID for the ConfigServer access point contains the hexadecimal digits in the same order as they are reported for the base MAC.

reported MAC

$ esptool.py --chip esp32 read_mac
esptool.py v3.0
Serial port /dev/ttyUSB0
Connecting........_____..
Chip is ESP32-D0WDQ6 (revision 1)
Features: WiFi, BT, Dual Core, 240MHz, VRef calibration in efuse, Coding Scheme None
Crystal is 40MHz
MAC: 4c:11:ae:8b:32:ac

SSID of the access point

$ nmcli --fields BSSID,SSID,CHAN device wifi
BSSID              SSID                         CHAN 
4C:11:AE:8B:32:AD  OpenBikeSensor-4C11AE8B32AC  1    

The MAC of the SoftAP (BSSID) is different to the base MAC id. An explanation can be found at https://espressif-docs.readthedocs-hosted.com/projects/esp-idf/en/stable/api-reference/system/system.html#mac-address

This fixes #315.

SubOptimal avatar Sep 10 '22 15:09 SubOptimal

Build failure reason is

ERROR: Project not found. Please check the 'sonar.projectKey' and 'sonar.organization' properties, \
the 'SONAR_TOKEN' environment variable, or contact the project administrator

SubOptimal avatar Sep 10 '22 17:09 SubOptimal

Thanks for this PR! Since it will change the SSID of the OBS that are currently in use, we should have some text prepared for the release notes. Consider that the users don`t (need to) know what a mac is but they might need to update the connection to the OBS via the mobile if this is used.

amandel avatar Sep 12 '22 15:09 amandel

Build failure reason is

ERROR: Project not found. Please check the 'sonar.projectKey' and 'sonar.organization' properties, \
the 'SONAR_TOKEN' environment variable, or contact the project administrator

this is because your branch build does not have access to the SonarQube credentials. This is a needed security measure, but we might skip the SonarQube analysis in this case.

amandel avatar Sep 12 '22 15:09 amandel

Thanks for this PR! Since it will change the SSID of the OBS that are currently in use,

Valid point. I did not considered this impact.

we should have some text prepared for the release notes. Consider that the users don`t (need to) know what a mac is but they might need to update the connection to the OBS via the mobile if this is used.

Maybe we could use the OpenBikeSensor-<MAC> SSID only for the initial SoftAP and after configuration offer a possibility to change this SSID (like proposed in #141). Especially in workshops I would see some benefit to have a known SSID, which is based on the MAC of the device, to connect to the expected device.

SubOptimal avatar Sep 12 '22 18:09 SubOptimal

ERROR: Project not found. ...

this is because your branch build does not have access to the SonarQube credentials. This is a needed security measure, but we might skip the SonarQube analysis in this case.

This means we would step always into this problem, when someone raise a PR from a fork?

Would it be possible to run the SonarQube analysis on a branch based on the PR instead on the downstream branch?

as draft (non complete list of commands, only to describe the principle)

$ git fetch origin pull/<PR-ID>/head:sonar_branch
$ git checkout sonar_branch
$ <run the SonarQube analysis>
$ git branch -d sonar_branch

SubOptimal avatar Sep 12 '22 18:09 SubOptimal

$ git branch -d sonar_branch

IHMO this is because if you can modify the build workflow you might gain access to the otherwise secret data. I've created #317 for this.

amandel avatar Sep 12 '22 18:09 amandel

Valid point. I did not considered this impact.

I wonder how we can get valid feedback. Otherwise, since we expect not to bad impact, we can just change it and see if we get negative feedback.

I would see some benefit to have a known SSID

If it is not to long, it is well shown in the display once the access point is started.

amandel avatar Sep 12 '22 19:09 amandel

Should we do this together with #141 to get a consistent solution?

amandel avatar Sep 15 '22 08:09 amandel