no keep alive support?

chrishamm

@Zambiorix I think both features are already supported in SBC mode and v3.5 will get an emulation layer for the rr_ style HTTP API as well (see here for the DSF RESTful API). In addition, DSF provides an IPC socket, so you may not even need to use HTTP if go that way.

~~We don't have plans to add keep-alive support to the firmware anytime soon and since we're short on sockets in standalone mode, I don't think we will add WebSocket support to the firmware either.~~ WebSockets can be used to receive object model updates in SBC mode, though.

Zambiorix

@chrishamm

I am not using duet3 in SBC mode, I am developing my own custom controller and want to remove layers of complexity, if possible.

Looking at the code, keep-alive is implemented, at least partially, in

src/Networking/HttpResponder.cpp

I have found several lines where Connection is set to close and other lines where a keepOpen variable is used to set close or keep-alive.

I also found a line where the socket is kept open

Commit(keepOpen ? ResponderState::reading : ResponderState::free, false);

And other places close it straight away

Commit(ResponderState::free, false);
//or
Commit();

In GetJsonResponse I have noticed, keepOpen is set to false and never changes.

bool HttpResponder::GetJsonResponse(const char *_ecv_array request, OutputBuffer *&response, bool& keepOpen) noexcept
{
	keepOpen = false;	// assume we don't want to persist the connection
...

To me all this looks like 90% of the keep alive support is already in place?
I would try to finish the implementation myself.

I could check for the existence of an extra custom header and, if present, keep the socket open. This would prevent browsers and other connections that send keep-alive from actually keeping the socket open.

The HttpReceiveTimeout is set to 2 seconds, so the socket will close automatically when not used.

I only need 1 connection for my project, so I could also try to implement that only 1 socket can be kept alive concurrently, the rest is ignored.

Do you know of any other pitfalls?
Keep-alive apparently has been tested in the past?

Thanks
Cheers

Gerd

chrishamm

@Zambiorix OK, so in fact it looks like it is already supported in the firmware. If a client sends a JSON request with Connection: keep-alive in the headers, it should keep the connection alive already. What board (base) are you using?

Zambiorix

@chrishamm

I am using a Duet 3 MB6HC v1.0

There is some support implemented, but not for every command.
(text/plain commands don't have it)

And currently keep-alive is disabled, so Connection header is always set to close.

It has been tested in the past, but not sure it has only been disabled due to resource use ... there could still be other issues.

I would allow keep-alive for following conditions:
#1: Connection request header is set to keep-alive
#2: a custom request header is present (so browsers and other connections that set keep-alive by default are locked out)
#3: only one keep-alive socket is active concurrently
#4: read timeout of 2 seconds is plenty enough to automatically close the socket

This way there is no impact for default use.

At the moment I have no development environment installed and I don't have much time right now, but I would definitely look into this in the future.

Zambiorix

@chrishamm

I have installed the development environment and implemented keep-alive for HttpResponder.

Its has no impact on browser calls, those sockets get closed even when Connection: keep-alive is set.

If the Rrf-Keep-Alive: force header is set, keep-alive starts working for a single socket. (so max 1 concurrent open socket)
__LPC17xx__ is also checked and, if set, disables keep-alive.

I have to do some more testing, but all seems to work smooth.

If interested I can create a pull request, for you to verify?

Cheers
Gerd

chrishamm

@Zambiorix Yes, sounds good, thanks. Please target the 3.5-dev branch if possible.

Zambiorix

@chrishamm

When moving to the 3.5-dev branch, I get the following build error:
unrecognized option '--no-warn-rwx-segment'

I'm using ARM toolchain version 10.3-2021.10

Should I use a different one?

Thanks
Cheers

Gerd

gloomyandy

@Zambiorix See: https://github.com/Duet3D/RepRapFirmware/wiki/Building-RepRapFirmware, if you are using a current 3.5-dev you are in effect working with 3.5 beta 3.

Zambiorix

@chrishamm

Ok, I had to install a more recent ARM toolchain (12.2) and after changing ArmGccPath and adding CrcAppender to the PATH, it builds.
But on generating the binary file, I get:

Generating binary file
arm-none-eabi-objcopy -O binary "/Users/gerd/Documents/Projects/Projects/Duet3D/RepRapFirmware/Duet3_MB6HC/Duet3Firmware_MB6HC.elf" "/Users/gerd/Documents/Projects/Projects/Duet3D/RepRapFirmware/Duet3_MB6HC/Duet3Firmware_MB6HC.bin" && CrcAppender "/Users/gerd/Documents/Projects/Projects/Duet3D/RepRapFirmware/Duet3_MB6HC/Duet3Firmware_MB6HC.bin"
Failure processing application bundle.
Bundle header version compatibility check failed.
A fatal error occured while processing application bundle
make[1]: [post-build] Error 159 (ignored)

any ideas?

Zambiorix

@gloomyandy thanks! Figured out I had to install a more recent arm toolchain