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

WiFiServer - deprecate available(), use accept() everywhere #8860

Merged
merged 2 commits into from
Nov 27, 2023

Conversation

JAndrassy
Copy link
Contributor

In esp8266 and esp32 Arduino WiFi library server.available() was never implemented as documented. It is implemented like server.accept() is documented by Arduino.

server.accept() was an alias for the server.available() from the beginning here in esp32 Arduino platform.

I think with the new major version 3 this should be done. esp8266 Arduino did this change for version 3 too.

esp8266 and esp32 server.available() is not implemented as documented
it is implemented like server.accept() is documented by Arduino
@SuGlider SuGlider self-assigned this Nov 26, 2023
@SuGlider SuGlider self-requested a review November 27, 2023 12:30
Copy link
Collaborator

@SuGlider SuGlider left a comment

Choose a reason for hiding this comment

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

LGTM - It still is back compatible by keeping the available() as it is, but with a warning for being deprecated.

More information about the argument that @JAndrassy has made:

Based on the documentation from https://www.arduino.cc/reference/en/libraries/wifi/server.available/
The method available() inherits from the Stream utility class. Therefore it should tell how many bytes are ready for reading.

The PR is correct in changing it to accept() which is how it was implemented.
https://www.arduino.cc/reference/en/libraries/ethernet/server.accept/

@SuGlider SuGlider added the Area: BT&Wifi BT & Wifi related issues label Nov 27, 2023
@SuGlider SuGlider added this to the 3.0.0 milestone Nov 27, 2023
@SuGlider SuGlider added the Status: Pending Merge Pull Request is ready to be merged label Nov 27, 2023
@SuGlider
Copy link
Collaborator

Thanks @JAndrassy !!

@me-no-dev me-no-dev merged commit 990e3d5 into espressif:master Nov 27, 2023
52 checks passed
@JAndrassy JAndrassy deleted the wifi_server_accept branch November 27, 2023 13:18
@mrengineer7777
Copy link
Collaborator

Beautiful work. Well done!

@JAndrassy
Copy link
Contributor Author

@JAndrassy
Copy link
Contributor Author

JAndrassy commented Nov 27, 2023

@SuGlider

The method available() inherits from the Stream utility class. Therefore it should tell how many bytes are ready for reading.

Server inherits from Print. It doesn't inherit from Stream.
server.available() is for a Server implementation which manages the connected clients. available() then returns first connected client which has data available. It can return the same client repeatedly.

documentation for `server.available()':

Gets a client that is connected to the server and has data available for reading. The connection persists when the returned client object goes out of scope; you can close it by calling client.stop().

My NetApiHelpers library has ArduinoWiFiServer for ESP32. This has correct available() and the print-to-all-clients function to really implement the Print base class. But most users don't need this. accept() is what they really need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BT&Wifi BT & Wifi related issues Status: Pending Merge Pull Request is ready to be merged
Projects
Development

Successfully merging this pull request may close these issues.

4 participants