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

Feature Request: TCP Keepalive in WiFiClientSecure #8939

Closed
6 tasks
mobizt opened this issue Jun 8, 2023 · 5 comments · Fixed by mobizt/Arduino#1
Closed
6 tasks

Feature Request: TCP Keepalive in WiFiClientSecure #8939

mobizt opened this issue Jun 8, 2023 · 5 comments · Fixed by mobizt/Arduino#1

Comments

@mobizt
Copy link
Contributor

mobizt commented Jun 8, 2023

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Platform

  • Core Version: latest (3.1.2)

Problem Description

The TCP Keepalive was already implemented in WiFiClient class but it was missing from WiFiClientSecure class.

Can you add support TCP Keepalive in WiFiClientSecure class?

void WiFiClient::keepAlive (uint16_t idle_sec, uint16_t intv_sec, uint8_t count)
{
_client->keepAlive(idle_sec, intv_sec, count);
}
bool WiFiClient::isKeepAliveEnabled () const
{
return _client->isKeepAliveEnabled();
}
uint16_t WiFiClient::getKeepAliveIdle () const
{
return _client->getKeepAliveIdle();
}
uint16_t WiFiClient::getKeepAliveInterval () const
{
return _client->getKeepAliveInterval();
}
uint8_t WiFiClient::getKeepAliveCount () const
{
return _client->getKeepAliveCount();
}

@earlephilhower
Copy link
Collaborator

WiFiClientSecure already implements this through the magic of C++ inheritance, no? The following snippet builds fine:

#include <ESP8266WiFi.h>

void setup() {
  WiFiClientSecure s;
  s.keepAlive(1,2,3);
}

void loop() { }

@mobizt
Copy link
Contributor Author

mobizt commented Jun 9, 2023

I test with your simple code and device crashed with this error in Serial monitor.

--------------- CUT HERE FOR EXCEPTION DECODER ---------------

Exception (28):
epc1=0x40207056 epc2=0x00000000 epc3=0x00000000 excvaddr=0x00000000 depc=0x00000000

>>>stack>>>

ctx: cont
sp: 3ffffe30 end: 3fffffd0 offset: 0150
3fffff80:  40207558 00000000 00001388 feefeffe  
3fffff90:  00000000 00000000 00000000 3ffef504  
3fffffa0:  3ffef504 3fff0e1c feefeffe feefeffe  
3fffffb0:  feefeffe feefeffe 3ffee6f8 40204510  
3fffffc0:  feefeffe feefeffe 3fffdab0 40100c39  
<<<stack<<<

--------------- CUT HERE FOR EXCEPTION DECODER ---------------

 ets Jan  8 2013,rst cause:2, boot mode:(3,6)

load 0x4010f000, len 3424, room 16 
tail 0
chksum 0x2e
load 0x3fff20b8, len 40, room 8 
tail 0
chksum 0x2b
csum 0x2b
v0005c970
~ld

And this is the decoded stack trace.

Exception 28: LoadProhibited: A load referenced a page mapped with an attribute that does not permit loads
PC: 0x40207056
EXCVADDR: 0x00000000

Decoding stack results
0x40204510: loop_wrapper() at C:\Users\User\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.1.2\cores\esp8266\core_esp8266_main.cpp line 255

I think WiFiClientSecure should provide the public functions to calling this private WiFiClientSecure context.

std::shared_ptr<WiFiClientSecureCtx> _ctx;

@mobizt
Copy link
Contributor Author

mobizt commented Jun 9, 2023

I have tested this before to make sure that we can't call keepAlive directly from WiFiClientSecure class. The error is the same as I post above.

I can call keepAlive from _ctx without error if I edit WiFiClientSecureBearSSL.h to make this public access.
std::shared_ptr<WiFiClientSecureCtx> _ctx;

That is why I request this feature here for WiFiClientSecure.

@earlephilhower earlephilhower reopened this Jun 9, 2023
@earlephilhower
Copy link
Collaborator

Hmm, could be an issue with object slicing.

In any case, it looks like you have a solution in mind and a test case. Why not make a PR with the change?

@earlephilhower earlephilhower added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jun 9, 2023
@mobizt
Copy link
Contributor Author

mobizt commented Jun 9, 2023

Ok, I will submit a PR for this.

@mobizt mobizt reopened this Jun 9, 2023
@earlephilhower earlephilhower removed the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jun 10, 2023
hasenradball pushed a commit to hasenradball/Arduino that referenced this issue Nov 18, 2024
* Add support WiFiClientSecure TCP KeepAlive
* Make TCP keepalive and related functions virtual.
* Make TCP keepalive and related functions override.

Fixes esp8266#8939
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 a pull request may close this issue.

2 participants