Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

make serveProgmem public, remove server.onNotFound(serveProgmem) from begin - compatibility Espalexa #119

Closed
wants to merge 2 commits into from

Conversation

zenonasz
Copy link

@zenonasz zenonasz commented Aug 4, 2021

Hi,

Could you please take a look?

My intention is to make the serveProgrem public so when another service is also running on the same port you can handle and filter requests on your main code.

In the specific case of Espalexa [https://github.com/Aircoookie/Espalexa.git]:

The library offers a check to determine if the request is for the Espalexa

server.onNotFound({
if (!espalexa.handleAlexaApiCall(server.uri(),server.arg(0)))
{
server.send(404, "text/plain", "Not found");
}
});

after the changes I proposed this can also be handled with esp8266-iot-framework like so:

GUI.server.onNotFound([](AsyncWebServerRequest *request)
{
if (!espalexa.handleAlexaApiCall(request)) //if you don't know the URI, ask espalexa whether it is an Alexa control request
{
GUI.serveProgmem(request);
}
});

Take a look and see if my changes serve the purpose.

Regards,

Zenon

…server.onNotFound so it can be handled by main code
@maakbaas
Copy link
Owner

maakbaas commented Aug 4, 2021

Removing the call from webServer.cpp in this pull request is of course problematic, since this will break loading the default GUI of the framework ;).

I am not sure if I completely understand what you are trying to do...

@aaronse
Copy link

aaronse commented Aug 4, 2021

Guessing Zenon wants framework extended to allow users to optionally configure requests (not handled by espframework dashboard web) to be handled by callbacks in their main code. Enabling Zenon in this case to integrate with ESPAlexa

@zenonasz
Copy link
Author

zenonasz commented Aug 4, 2021

Removing the call from webServer.cpp in this pull request is of course problematic, since this will break loading the default GUI of the framework ;).

I am not sure if I completely understand what you are trying to do...

Using it for more than a month now, works as it should.

@zenonasz
Copy link
Author

zenonasz commented Aug 4, 2021

Guessing Zenon wants framework extended to allow users to optionally configure requests (not handled by espframework dashboard web) to be handled by callbacks in their main code. Enabling Zenon in this case to integrate with ESPAlexa

Exactly that.

@maakbaas
Copy link
Owner

maakbaas commented Aug 4, 2021

Removing the call from webServer.cpp in this pull request is of course problematic, since this will break loading the default GUI of the framework ;).
I am not sure if I completely understand what you are trying to do...

Using it for more than a month now, works as it should.

yes, but you have this code:

GUI.server.onNotFound([](AsyncWebServerRequest *request)
{
if (!espalexa.handleAlexaApiCall(request)) //if you don't know the URI, ask espalexa whether it is an Alexa control request
{
GUI.serveProgmem(request);
}
});

which is not part of the pull request and re-implements the deleted functionality. If you would remove this, and try purely with the framework it will not work. so another mechanism is needed to tie into the onNotFound function while keeping existing functionality inside of the framework.

@zenonasz
Copy link
Author

zenonasz commented Aug 4, 2021

Removing the call from webServer.cpp in this pull request is of course problematic, since this will break loading the default GUI of the framework ;).
I am not sure if I completely understand what you are trying to do...

Using it for more than a month now, works as it should.

yes, but you have this code:

GUI.server.onNotFound([](AsyncWebServerRequest *request)
{
if (!espalexa.handleAlexaApiCall(request)) //if you don't know the URI, ask espalexa whether it is an Alexa control request
{
GUI.serveProgmem(request);
}
});

which is not part of the pull request and re-implements the deleted functionality. If you would remove this, and try purely with the framework it will not work. so another mechanism is needed to tie into the onNotFound function while keeping existing functionality inside of the framework.

You are right, Examples won't work unless the requests are forwarded using GUI.serveProgmem(request); from the main code.
i would appreciate your input though, I'm sure Espalexa is not the only integration that needs this change

@maakbaas maakbaas changed the base branch from master to 1.9.0-dev November 6, 2021 12:21
@maakbaas maakbaas closed this in 7d033c2 Nov 6, 2021
@maakbaas
Copy link
Owner

maakbaas commented Nov 6, 2021

Sorry for the long delay.

I have implemented this functionality in another way in the commit above, which will be part of the next release.

The documentation is updated accordingly.

maakbaas added a commit that referenced this pull request Nov 6, 2021
maakbaas added a commit that referenced this pull request Nov 6, 2021
* Add requestHandler (fixes #119) (#125)

* Add optionLabels (#120)

* Updated generated HTML and docs

* Bump version

Co-authored-by: benklop <benklop@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants