-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
Adding softAP SSID & PSK query API #4138
Conversation
Signatures: String ESP8266WiFiAP::softAPSSID() const; String ESP8266WiFiAP::softAPPSK() const;
String ESP8266WiFiAPClass::softAPPSK() const { | ||
struct softap_config config; | ||
wifi_softap_get_config(&config); | ||
return String(reinterpret_cast<char*>(config.password)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, this could cause an issue. When a 64-byte psk is used, it is not 0-terminated, and String assumes a null-terminator.
Instead, a char psk[65] should be declared, config.password should be memcpy()d into it, and then psk[64] = 0;. Then, return String(psk).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@devyte
Yes That's right.
This is the String
constructor:
String::String(const char *cstr) {
init();
if(cstr)
copy(cstr, strlen(cstr));
}
} | ||
|
||
/** | ||
* Get the configurd(Not-In-Flash) softAP PSK or PASSWORD. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor typo -> configured.
String ESP8266WiFiAPClass::softAPSSID() const { | ||
struct softap_config config; | ||
wifi_softap_get_config(&config); | ||
char* name = reinterpret_cast<char*>(config.ssid); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just
config.ssid[sizeof(config.ssid) - 1] = 0; // guard
return String((const char*)(void*)config.ssid) ?
String(char*) will anyway search for the last \0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK sorry I did not see the hidden-outdated comment.
Signatures: static bool IPAddress::isValid(const String& arg); static bool IPAddress::isValid(const char* arg, size_t len);
@@ -47,6 +47,9 @@ class ESP8266WiFiAPClass { | |||
uint8_t* softAPmacAddress(uint8_t* mac); | |||
String softAPmacAddress(void); | |||
|
|||
String softAPSSID() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation on this line is different from nearby lines
* @return String SSID. | ||
*/ | ||
String ESP8266WiFiAPClass::softAPSSID() const { | ||
struct softap_config config; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation in this function is different from nearby functions
cores/esp8266/IPAddress.cpp
Outdated
@@ -112,4 +112,51 @@ String IPAddress::toString() const | |||
return String(szRet); | |||
} | |||
|
|||
/* ref: https://code.woboq.org/userspace/glibc/resolv/inet_pton.c.html#inet_pton4 */ | |||
static bool inet_pton4(const char *start, const char *end) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this function different from IPAddress::fromString(), found here? If they serve equivalent purpose, I'd rather not duplicate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@devyte
So sorry for late reply.
You say we should modify bool IPAddress::fromString(const char *address)
function for adding validate support ? without duplicate
Because current IPAddress public API doesn't support validating IPV4 as string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@devyte
My suggestion:
1.Adding a private validating method:
static bool IPAddress::isValid(const char* arg, size_t len);
2.Adding correspond public method:
static bool IPAddress::isValid(const String& ip);
3.Reuse method of 1 inside 2 and bool IPAddress::fromString(const char *address)
function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IMAN4K said:
Because current IPAddress public API doesn't support validating IPV4 as string
Are you saying that the following does not validate a string?
bool isvalid = IPAddress().fromString("17604123874106239");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@devyte
Yes that's correct, my mistake!
But we need to make an object for every validation.
It's better with a public static member(with good api name isValid
rather than fromString
), then every where in code:
if (IPAddress::isValid(...)) ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The object constructed is 4 bytes in size, essentially one int, which is a temporary instantiated on the stack and immediately destroyed, so that's not a problem (btw, destruction is trivial).
What I suggest is a static convenience method like you propose, but something like this:
bool IPAddress::isValid(const char *str)
{
return IPAddress().fromString(str);
}
bool IPAddress::isValid(String &str)
{
return IPAddress().fromString(str);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok.
I'll make changes very soon.
@IMAN4K will you be replying to this? |
cores/esp8266/IPAddress.cpp
Outdated
@@ -112,4 +112,51 @@ String IPAddress::toString() const | |||
return String(szRet); | |||
} | |||
|
|||
/* ref: https://code.woboq.org/userspace/glibc/resolv/inet_pton.c.html#inet_pton4 */ | |||
static bool inet_pton4(const char *start, const char *end) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The object constructed is 4 bytes in size, essentially one int, which is a temporary instantiated on the stack and immediately destroyed, so that's not a problem (btw, destruction is trivial).
What I suggest is a static convenience method like you propose, but something like this:
bool IPAddress::isValid(const char *str)
{
return IPAddress().fromString(str);
}
bool IPAddress::isValid(String &str)
{
return IPAddress().fromString(str);
}
signatures: static bool isValid(const String& arg); static bool isValid(const char* arg);
Signed-off-by: Iman Ahmadvand [email protected] (github: @IMAN4K)