Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change byte order of base MAC in the SoftAP SSID. #316

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

SubOptimal
Copy link
Member

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
Copy link
Member Author

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

@amandel
Copy link
Member

amandel commented Sep 12, 2022

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
Copy link
Member

amandel commented Sep 12, 2022

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.

@SubOptimal
Copy link
Member Author

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
Copy link
Member Author

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

@amandel
Copy link
Member

amandel commented Sep 12, 2022

$ 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
Copy link
Member

amandel commented Sep 12, 2022

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
Copy link
Member

amandel commented Sep 15, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MAC address for the access point name is in reversed byte order
2 participants