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

Berry web_add_handler called before Webserver is initialized #21272

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

s-hadinger
Copy link
Collaborator

Description:

Bug introduced with BrRestart. There is a mechanism to detect if web_add_handler was missed because of a BrRestart. It checked if network was up, but failed to check if Webserver was also created.

In some cases, the event would be fired too soon, before Webserver is created, hence lost.

Related issue (if applicable): fixes #21158

Checklist:

  • The pull request is done against the latest development 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 with Tasmota core ESP8266 V.2.7.6
  • The code change is tested and works with Tasmota core ESP32 V.3.0.0
  • I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

@s-hadinger s-hadinger merged commit 469492a into arendst:development Apr 25, 2024
59 checks passed
@storchalberto
Copy link

I have used benzino77 (developer) to build my version. The version i see is 13.4.1.2. The web_add_handler seems to function well but now the when the browser interrogate the page, it stay waiting the end of page transfer. It seems to have problem the webserver.content_flush(): the browser receive the page but do not terminate the connection. Also, during this, the main page do not respond. If i force the browser to close my page, the main page return to function. This not happens using the standart 13.4.0 version.

@s-hadinger
Copy link
Collaborator Author

You should call webserver.content_stop() when the page is finished.

Unless you have specific needs, webserver.content_flush() is generally not needed

@storchalberto
Copy link

The webserver.content_stop() after the webserver.content_send of the page add at the end of the page a new tags, with your copirights and so the page is not correct. in the version 14.4.0 the webserver.content_flush() do not do it, end the page and function correctly

@s-hadinger
Copy link
Collaborator Author

Good point. Do you have a test case?

Maybe we should add webserver.content_close() to force the end of the page

@storchalberto
Copy link

the webserver.content_close() do the same. the connection to the browser is not ended

@storchalberto
Copy link

storchalberto commented Apr 25, 2024

Here the code for test:

import webserver
class Web_page_demo : Driver
  def page_say_test1()
    if !webserver.check_privileged_access() return nil end
	var fr=open("test1.htm", 'r')
	var mybuff=fr.read()
	fr.close()
	webserver.content_send(mybuff)
	webserver.content_flush()
  end
  def init()
	print("init")
	tasmota.add_driver(self)
  end
  def web_add_handler()
	webserver.on("/test1.htm", self.page_say_test1)
	print("page added")
  end 
end
web_page_demo_instance = Web_page_demo()

Here the test page (test1.htm):

<html>
<body>
Test page
</body>
</html>

If you replace the webserver.content_flush() with the webserver.content_stop() the page in the browser is completed, vbut not correct due for the addition of your copyright at the end. in the 13.4.0 version it runs correctly

@s-hadinger
Copy link
Collaborator Author

I could reproduce, thanks. I suspect it is due to new Arduino framework Core3 that has a slower reuse of non-closed TCP connections.

I'm adding webserver.content_close() to solve this

@s-hadinger s-hadinger mentioned this pull request Apr 25, 2024
6 tasks
hawa-lc4 pushed a commit to hawa-lc4/Tasmota-dev that referenced this pull request May 7, 2024
@s-hadinger s-hadinger deleted the fix_web_add_handler branch January 6, 2025 14:24
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.

2 participants