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

Improve WiFi scan result display #10253

Closed
wants to merge 4 commits into from

Conversation

DL6ER
Copy link

@DL6ER DL6ER commented Dec 25, 2020

Description:

Checklist:

  • The pull request is done against the latest dev branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • The code change is tested and works on Tasmota core ESP8266 V.2.7.4.9
  • The code change is tested and works on Tasmota core ESP32 V.1.0.5-rc4
  • I accept the CLA.

I cannot test ESP32 myself, however, compilation passes fine.

This improves WiFi scan results:

  • Reveal if multiple AP have the same SSID but different signal strengths and show them group them together
  • Color-code the signal quality to improve accessibility

Exemplary screenshots:

Screenshot from 2020-12-25 14-06-28

Further less important details like encryption type and RSSI are hidden in a tooltip, like
Screenshot from 2020-12-25 14-07-14

Signed-off-by: DL6ER <dl6er@dl6er.de>
@s-hadinger
Copy link
Collaborator

This is a nice feature but I'm not in favor of spending precious flash space for a cosmetic feature used only once in the lifetime of the device.

Even if we were to add the feature, avoid float calculation as much as possible. It is very expensive in code size.

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER force-pushed the tweak/wifi_scan_results branch from 41e35af to 1d4a341 Compare December 25, 2020 18:03
@DL6ER
Copy link
Author

DL6ER commented Dec 25, 2020

Current development:

RAM:   [======    ]  57.2% (used 46832 bytes from 81920 bytes)
Flash: [======    ]  58.4% (used 597500 bytes from 1023984 bytes)

My branch:

RAM:   [======    ]  57.2% (used 46832 bytes from 81920 bytes)
Flash: [======    ]  58.4% (used 597692 bytes from 1023984 bytes)

If we leave off the description of the WiFi encryption type (because it shouldn't really matter), we get my new scanning display at a mere 96 bytes of extra space:

RAM:   [======    ]  57.2% (used 46832 bytes from 81920 bytes)
Flash: [======    ]  58.4% (used 597596 bytes from 1023984 bytes)

@DL6ER
Copy link
Author

DL6ER commented Dec 25, 2020

The failed CI run seems to be a mistake of the CI itself:

Error: read ECONNRESET

Copy link
Collaborator

@s-hadinger s-hadinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With both suggested changes, you should be at only +64 bytes.

delay(0);

int j = 0;
String nextSSID = "";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can save code with
String nextSSID;

You don't need to give it a value, is it still initialized.

Comment on lines 2136 to 2142
if(quality > 50) {
// Scale red component to go from yellow to green (full green)
colors[0] = (0xFF * (100-quality))/50;
} else {
// Scale green component to go from red to yellow (full red)
colors[1] = (0xFF * quality)/50;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can save even more here:

            if(quality > 50) {
              // Scale red component to go from yellow to green (full green)
              colors[0] = changeUIntScale(quality, 50, 100, 0xFF, 0);
            } else {
              // Scale green component to go from red to yellow (full red)
              colors[0] = changeUIntScale(quality, 0, 50, 0, 0xFF);
            }

I would also suggest to avoid pure Yellow #FFFF00 which is not readable with light background. Maybe going for a darker version would still do it:

            uint8_t colors[2] = { 0xAA, 0xAA };
            if(quality > 50) {
              // Scale red component to go from yellow to green (full green)
              colors[0] = changeUIntScale(quality, 50, 100, 0xAA, 0);
            } else {
              // Scale green component to go from red to yellow (full red)
              colors[0] = changeUIntScale(quality, 0, 50, 0, 0xAA);
            }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured that most (all?) will run Tasmota in the standard configuration which seems to be a black background. This seems to be a wrong assumption and getting colors what are visible for all (e.g. green or orange background) is hard if not impossible.

Removing the quality colors isn't much of a loss as the results are anyway sorted by RSSI (= quality) and I still think my improvement brings advantage to the display. Removing the color code, we're down to 597492 bytes so even less (8 bytes) than current development.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative - and color-independent - representation would be using HTML5 progress-bar
Screenshot from 2020-12-26 09-14-21

I haven't pushed this so far, but it'd come at 597536 bytes (+36 compared to development). As said before, my change without color or progress-bar comes at 597542 bytes (-8 compared to development).

@arendst
Copy link
Owner

arendst commented Dec 25, 2020

As said in the original issue I won't do changes regarding preferred (b)ssid. This change is only half of what needs to be changed as the command line part has not been tackled.

Changing that too will increase code while current functionality is good enough for most users.

As it stands I won't merge but I will keep it in mind.

@DL6ER
Copy link
Author

DL6ER commented Dec 26, 2020

This change is not meant at all into the direction of preferred BSSID, it just helps users to get a better picture of their WiFi neighborhood at the exact location of the Tasmota device. Hence, I also felt no need to change the command line part.

Concerning BSSID, I can see SetOption56/57 is the preferred choice, even when I cannot use it right now as it leads to malfunction on my side (but that's a separate issue, #10254).

… with arbitrary background colors.

Signed-off-by: DL6ER <dl6er@dl6er.de>
@arendst
Copy link
Owner

arendst commented Dec 26, 2020

No offence at all but did you notice you are using the same channel on equal ssids? The general approach is to NOT have the same channels on near APs. They should not overlap as if they do they interfere eachother.

If you manage to change them you'll see tasmota will much easier switch to the strongest signal AP.

@DL6ER
Copy link
Author

DL6ER commented Dec 26, 2020

I have the very widely used Fritz!Box in its "Mesh" configuration. I noticed this defect as well, however, the manufacturer says that it is expected. I set both 2.4 GHz and 5GHz to "auto-channel". As result, the 2.4 GHz WiFi are all on the same channel. The interesting bit to note is that the 5GHz are all on different channels... And, of course, the channels can only be configured at the "master", not allowing any manual intervention as to what channels the individual APs use.

Feel free to close the PR if this is not where you think the project should be going. I'll rather concentrate on features rather than UI tweaks in the future (even when we have a variant right now that uses even less Flash than the current development build ;-)).

arendst added a commit that referenced this pull request Dec 27, 2020
Add BSSID and Signal Strength Indicator to GUI wifi scan result (#10253)
@arendst
Copy link
Owner

arendst commented Dec 27, 2020

In the end it became this:

image

at the cost of over 500 bytes (!) but I would like to see the Signal Strength Indicator. It can be disabled by removing #define USE_ENHANCED_GUI_WIFI_SCAN (as I use it myself ;-)

Thx for the idea.

NB. To not needing translations and screwing up GUI I do not display texts like "Channel" or "Quality". The Italians tend to use abnormal long names for simple things ;-)

@arendst arendst closed this Dec 27, 2020
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.

3 participants