Refactoring differences

[CapsAdmin]

@vwout thought I'd ask for forgiveness rather than permission!

I've refactored the whole thing, so if you find the time to upgrade, for obs-visca-control you'd need to replace

socket.find_first_address("*", port, {family="inet", socket_type="dgram", protocol="udp"})

with

socket.find_first_address_info("*", port, nil, "inet", "dgram", "udp")

Other than that it should be the same.

addressinfo is its own object now which could also be addressinfo:free()'d. socket:receive_from() also returns a proper addressinfo object. In the past it was just some adhoc table that looked like one.

But the way the old socket:recive_from() was implemented, it would likely cause a memory leak when called.

I also removed the docker, action and lunit stuff, not necessarily because I didn't like it, but just because I didn't understand how to set it up. I don't mind this being added back again as long as there are clear instructions on how to setup. So right now it just builds luajit from source and runs test/run.lua

[vwout]

No problem; I'll give this a try and test it later (quite busy at the moment).
It's quite some code to go through. I see you simplified the unit test; not a problem per se, but you also removed some checks (linting, byte-code checking) and you are now using lua 5.1 rather than 5.2 (would be best to test both, optionally also 5.3 and 5.4).

[CapsAdmin]

No problem; I'll give this a try and test it later (quite busy at the moment). It's quite some code to go through. I see you simplified the unit test; not a problem per se, but you also removed some checks (linting, byte-code checking) and you are now using lua 5.1 rather than 5.2 (would be best to test both, optionally also 5.3 and 5.4).

I was thinking that this would target LuaJIT only, as it doesn't make sense to me to target Lua5.3 and Lua5.4 with an ffi module.
My thinking is that once you're using vanilla Lua along with Lua modules, you're most likely using luasocket anyway.

[CapsAdmin]

#11

Hi, good that you did not merge this change. As far as I know, timeouts only occur when the socket is blocking. Other than that I'd rather not introduce multiple returns in a function when not strictly needed.

Well that's kind of funny, I completely misunderstood..

What I was actually thinking was that "timeout" meant you should try again, like a generic resource is busy error message. But I believe we should have both then, one generic "timeout" error for blocking mode, and one generic "waiting" for non blocking mode.

So because I merged this already, "timeout" should be replaced with "waiting" and then I should add the logic back for "timeout" in blocking mode.

[CapsAdmin]

I've fixed the poll api and added socket:poll(timeout, ...flags), as well as fixed the timeout with blocking sockets.

So there are 2 ways to deal with this now.

One is the blocking socket way as you mentioned:

local sock = assert(socket.create("inet", "stream", "tcp"))
assert(sock:connect("httpbin.org", "http"))
assert(sock:send("GET /delay/10 HTTP/1.1\r\nHost: httpbin.org\r\n\r\n"))
assert(sock:set_option("rcvtimeo", 250))
local res, err = sock:receive()
if res then
    error("expected timeout error, got data")
end
eq(err, "timeout")

another one is using poll:

local sock = assert(socket.create("inet", "stream", "tcp"))
assert(sock:connect("httpbin.org", "http"))
assert(sock:send("GET /delay/10 HTTP/1.1\r\nHost: httpbin.org\r\n\r\n"))
assert(sock:set_blocking(false))
sock:poll(250, "in")
local res, err = sock:receive()
if res then
    error("expected timeout error, got data")
end
eq(err, "timeout")

and the third would be the existing way i did it, by setting sock:set_blocking(false) and calling sock:receive(), in the examples above it would return nil, "tryagain", so it's up to you if you wan to sleep or use it in some sort of event loop somewhere.

I removed sock:settimeout() in favor of calling set_option directly.