Robot 19
TCP. I think I know what I did wrong yesterday. If I can make this work, it’ll help me TDD the server’s main loop. Bonus: Kitty!
I think I’d do better, by which I mean create working code faster, if I did TDD more than I do. Two things hold me back from that. First, sometimes it’s just that I can’t see how to TDD something. Second, I can see the code, and so I just write it.
The first case is almost invariably a sign that my code is too complicated, and I would do better to treat difficulty writing a test as a cry from the code to be better organized.
The second case is a bad habit, no more. It’s good when we know what to code: that doesn’t mean we should just type it in. It’s good to know what the wall should look like: that doesn’t mean we should just start nailing up boards without a level and some scaffolding.
In the case of the Robot World Server, the World bit, I want to do TDD, because the code needs to loop over all the clients, to look for any new clients arriving. It needs never to block, it needs to handle errors, and it may even need to use coroutines to implement threading, though I am pretty sure I can avoid that last.
Now maybe I can see a way to test the loops without real TCP objects in there, but it would involve a lot of fake TCP things. So I want more TCP experience even if I can’t use them directly, because I need to understand how they work.
Yesterday, I couldn’t make two clients and one server all in the same program. Today, I think I see my mistake, and plan to find out.
The Test That’s Red
I left a test red yesterday, in my TCP spike. I was tired and wanted to rest and think, so I left it red. No problem with that: it’s an experiment. And, rubber-ducking with some of my Slack pals, I think I saw what’s at issue.
Here’s the red test:
_:test("Multiple clients not round robin", function()
local server = getLocalServer(0)
local tcp = getTCP(0.01) -- can't be zero
local conn1 = tcp:connect(getMyIp(),8888)
_:expect(conn1).isnt(nil)
local conn2, error = tcp:connect(getMyIp(),8888)
_:expect(conn2, "conn2 "..error or "").isnt(nil)
end)
What I was trying to do was to get two connections to the local server, so that I could send a few messages, gain more familiarity, and perhaps even begin to TDD my server loop. But that code fails the final expectation with the message “already connected”.
My mistake was in thinking that tcp:connect is a function returning a connection. It is not. It is a function on a master that converts it to a connection, Thus, tcp and conn1 are the same object, so the second call to connect is equivalent to conn1:connect, which is clearly an error.
I think I was wrong to write the getTCP function so soon, as well. So let’s recode that test.
Try Again …
_:test("Multiple clients not round robin", function()
local ip = getMyIp()
local server = getLocalServer(0)
local socket = require("socket")
local master1 = assert(socket:tcp())
master1:connect(ip,8888)
local conn1 = master -- transformed~
_:expect(conn1).isnt(nil)
local master2 = assert(socket:tcp())
master2:connect(ip,8888)
local conn2 = master2
_:expect(conn2, "conn2 "..error or "").isnt(nil)
end)
Here I create two masters separately, with separate calls to tcp Each time, I convert them to connections with connect and then, for clarity, assign the transformed master to a name connoting connection.
I have not run the test. I wanted you to share the tension. I am hopeful that it will run this time. Let’s find out.
I am doomed to disappointment:
5: Multiple clients not round robin --
Actual: nil,
Expected: nil
5: Multiple clients not round robin -- Tests:77: attempt to concatenate a function value (global 'error')
Hm. The first did not connect. My test transliteration is faulty in a few ways, not least that I lost the error info. The function connect does return results.
_:test("Multiple clients not round robin", function()
local ip = getMyIp()
local server = getLocalServer(0)
local socket = require("socket")
local master1 = assert(socket:tcp())
local ok1, error1 = master1:connect(ip,8888)
_:expect(ok1,"conn1 "..error1 or "").is(1)
local master2 = assert(socket:tcp())
local ok2,error2 = master2:connect(ip,8888)
_:expect(ok2, "conn2 "..error2 or "").is(1)
end)
Simplified, separate error variables to make the symmetry more clear. Hopeful. Let’s try it.
5: Multiple clients not round robin -- Tests:71: attempt to concatenate a nil value (local 'error1')
Interesting. I must need parens:
_:expect(ok1,"conn1 "..(error1 or "")).is(1)
And same for conn2. Test. Test runs. Woot! The errors were in the concatenation .. which (of course) happens on the call to expect, not just in the obviously very unlikely and remote possibility of a mistake on my part.
Test runs!! So my insight yesterday about the master being transformed and thus I need a new tcp was accurate. (Of course there may still be things I missed, but clearly I grasped the error closest to the surface.
Let’s rename this test, close out objects, and then write a longer test to do some sending and receiving.
_:test("Can open two connections", function()
local ip = getMyIp()
local server = getLocalServer(0)
local socket = require("socket")
local master1 = assert(socket:tcp())
local ok1, error1 = master1:connect(ip,8888)
_:expect(ok1,"conn1 "..(error1 or "")).is(1)
local master2 = assert(socket:tcp())
local ok2,error2 = master2:connect(ip,8888)
_:expect(ok2, "conn2 "..(error2 or "")).is(1)
server:close()
master1:close()
master2:close()
end)
I remembered, this time, to close the things I had opened. I can see that I’m going to need either better habits than I now have, or some toolwork, to be sure that I close things.
I’ll set that aside for now. I want a test just like the one we just did, to build forward from.
Now I believe, but am not certain, that the connect function may not actually try to connect to the server until we send. Let’s check that first.
_:test("Server sees connect right after 'connect'", function()
local ip = getMyIp()
local server = getLocalServer(0)
local socket = require("socket")
local _nil, timeout = server:accept()
_:expect(timeout).is("timeout")
local master1 = assert(socket:tcp())
local ok1, error1 = master1:connect(ip,8888)
_:expect(ok1,"conn1 "..(error1 or "")).is(1)
local sConn1, error = server:accept()
_:expect(error).is(nil)
end)
I was mistaken! Glad I checked that, it’s probably useful to know. Server sees the connect, but need not accept it for the client connect to return OK. So the connection is somewhere in the library waiting, I guess.
Let’s try two. Oops, I almost forgot these:
server:close()
master1:close()
This is hard for me to remember. I could do things in the test’s before and after. Should figure something out. Not going to: going to press forward on the main issue, understanding TCP.
Here’s my first cut:
_:test("Two connections talking", function()
local ip = getMyIp()
local server = getLocalServer(0)
local socket = require("socket")
local master1 = assert(socket:tcp())
local ok1, error1 = master1:connect(ip,8888)
_:expect(ok1,"conn1 "..(error1 or "")).is(1)
local client1 = master1
local master2 = assert(socket:tcp())
local ok2,error2 = master2:connect(ip,8888)
_:expect(ok2, "conn2 "..(error2 or "")).is(1)
local client2 = master2
local serverC1,sError1 = server:accept()
_:expect(sError1).is(nil)
local serverC2,sError2 = server:accept()
_:expect(sError2).is(nil)
server:close()
master1:close()
master2:close()
end)
This story is getting long. I am creating two clients and renaming them to client1 and 2. I then try two server accepts expecting them both to succeed. Let’s try it.
Good so far. Continue the test.
client1:send("I am c1\n")
local msg
msg = serverC1:receive()
_:expect(msg).is("I am c1")
Works. Extend:
client1:send("I am c1\n")
local msg
msg = serverC1:receive()
_:expect(msg).is("I am c1")
serverC1:send("hi c1\n")
local c1Msg, c1Err = client1:receive()
_:expect(c1Msg).is("hi c1")
_:expect(c1Err).is(nil)
Test all runs. Honestly, I think I’ve learned all I need to here, but let’s do one more little thing:
client1:send("bye\n")
client2:send("never mind\n")
msg,err = serverC2:receive()
_:expect(err).is(nil)
_:expect(msg).is("never mind")
msg,err = serverC1:receive()
_:expect(err).is(nil)
_:expect(msg).is("bye")
I send two messages, one from each client, then receive them in the opposite order. I expect no hassle over this.
Test runs. One more test that I could imagine doing would be to send more than one message on the same channel. I feel sure that that will work, so I’ll defer that for now.
Time to think, and maybe to stop. I’ve been working for an hour and haven’t even performed my morning um rituals. Instead, I went right to the computer after feeding the cat.
Reflection
I feel that I have a pretty decent understanding of how to use the LuaSocket stuff. I think that in the real system, our Robot can just do a send followed by an immediate receive, with some finite timeout. I don’t see a need for threading or coroutines there, so long as there’s just one robot.
On the server side, we need to continually look for connections and for messages on all the connections we already have. Assuming that all our replies are fast, I see no need for threading on that side, but the loop will be a bit tricky if we don’t want to spin the CPU. There is a select function that is supposed to block until any of whatever’s in the select have come up with something to do. I think, but am not certain, that we can put the server itself in the list, not just clients.
However, if I want my Making App, which is essentially what I’m working with now, to actually use sockets, it would require some pretty sophisticated multi-tasking, because on the one side we’d have the robot, occasionally doing a quick send-receive, but on the other, world side, a bit of code that wants to do a receive-send. So that code is either hanging on a receive, which would block the robot, or there is some kind of co-routine thing going on … or …
Well, I saw a trick yesterday. Some wizard hooked the Codea tween clocktick function, which runs once per frame, and did some work there. But no, I really don’t think that’s a good idea.
I think the good idea is this: we’ll declare victory and move on. I think I can make the case that we understand TCP “well enough” to return our focus to the game. In order to make progress toward the eventual TCP hookup, we’ll work toward changing the way Robot and World interact now, via direct calls, into a message-oriented communication using the JSON / dictionary stuff in the Robot Worlds spec. We’ll need all that, of course, and when we have the whole game, or most of it, running using all the message/JSON stuff, we’ll be just a short TCP trip away from full client/server. At that point, maybe I’ll leave the rest of the server code to the reader, or maybe I’ll do it. I suspect the former, because I have no decent way to build a real server. If I had a compatible Codea for the Mac … but no. Not likely to do that.
So. Declaring victory on the major effort of understanding TCP. A good result, especially since yesterday I was stumped about parts of it, due to a serious misunderstanding, now resolved.
Commit TCP: Can open multiple connections and a server all in one app.
Remaining to be done is something important enough to start a new section about. Er, about which to start a new section … anyway:
Proper Usage
What I have in these tests looks a lot like the typical example you’d find on the web or in an actual book, a bunch of open code all in a row. Mine at least has some assertions, but it’s still very flat:
_:test("Two connections talking", function()
local ip = getMyIp()
local server = getLocalServer(0)
local socket = require("socket")
local master1 = assert(socket:tcp())
local ok1, error1 = master1:connect(ip,8888)
_:expect(ok1,"conn1 "..(error1 or "")).is(1)
local client1 = master1
local master2 = assert(socket:tcp())
local ok2,error2 = master2:connect(ip,8888)
_:expect(ok2, "conn2 "..(error2 or "")).is(1)
local client2 = master2
local serverC1,sError1 = server:accept()
_:expect(sError1).is(nil)
local serverC2,sError2 = server:accept()
_:expect(sError2).is(nil)
client1:send("I am c1\n")
local msg,err
msg,err = serverC1:receive()
_:expect(msg).is("I am c1")
_:expect(err).is(nil)
serverC1:send("hi c1\n")
local c1Msg, c1Err = client1:receive()
_:expect(c1Msg).is("hi c1")
_:expect(c1Err).is(nil)
client1:send("bye\n")
client2:send("never mind\n")
msg,err = serverC2:receive()
_:expect(err).is(nil)
_:expect(msg).is("never mind")
msg,err = serverC1:receive()
_:expect(err).is(nil)
_:expect(msg).is("bye")
serverC1:close()
serverC2:close()
server:close()
master1:close()
master2:close()
end)
Possibly, code like that shows us just what makes up each step. It’s not,however, very good code. And, very unfortunately, when we look at the applications people build using some facility like this one, their code looks a lot like the test above, all open steps, one after another after another after another.
We know better than this, don’t we? You can see by the white space in the test above that I have some kind of meaning in mind, each little bit making some bit of sense to me. A certain kind of programmer would put a comment in front of each one. Another kind, more the kind I’d like to be, would create a function or method for each of those chunks, with a meaningful name instead of a comment.
If we care for our fellow teammates and for ourselves, we would not leave production code looking like that. And yet, coaching teams, I have seen code like that all too often. It’s probably a case of what Hill calls “RORA”. To quote him from Twitter:
RORA means “Runs Once, Run Away”. It’s not one behavior, but a whole cluster, and the theme of the cluster is stopping the work in one part of the forest to go to another part, even though all that initial work does is barely pass the “customer saw it run and said okay” test.
Here’s a link to his whole thread on RORA.
This behavior is usually a result of the developers feeling pressure great enough to induce them to take shortcuts. It’s a bad habit even so, because it will slow us down, often as early as tomorrow when we have to change or replicate that code. But it’s hard—ain’t it hard—to do the right thing when we’re under pressure.
We can forgive ourselves, so we owe it to everyone to forgive the other person. And we need to find ways to help ourselves and the other person to do the best thing, even under pressure.
ANYWAY, that code needs to be packaged into something clear, clean, and sensible, and if and when I move beyond tests in a Spike, I’ll do that. I promise, not least because I have no intention of moving beyond these tests, but also because that’s how I roll in these articles.
And it is an interesting problem and I might come back to it. But we’ve got to get the robots rolling.
TDD Remarks:
I think of what I’ve done in this spike as “TDD”, because much of my thinking is the same as when I develop production code with TDD. But it would perhaps better be called “Test-Driven Learning And Recording Thereof”. Well, maybe not. But that’s what it is. And the TCP tests are a decent example of me doing that fairly well. I’m sure my betters do, well, better, but I think I’ve broken out the learning fairly well. I’ll include all the tests as an appendix to this article.
For today: excellent learning and correction of misunderstanding of TCP. Very pleasant.
See you next time!
Appendix: The Tests
-- RJ 20220608
-- DUPLICATE these then edit.
function test_TCP()
_:describe("TCP()", function()
_:before(function()
end)
_:after(function()
end)
_:test("Get my IP", function()
local socket = require("socket")
local myIp = getMyIp()
print(myIp)
end)
_:test("Server socket", function()
local myIp = getMyIp()
local socket = require("socket")
local server = getLocalServer()
local sIp, sPort = server:getsockname()
_:expect(sIp).is(myIp)
_:expect(sPort).is("8888")
server:close()
end)
_:test("Client connection returns nil if no server", function()
local myIp = getMyIp()
local socket = require("socket")
local tcp = assert(socket.tcp())
local result = tcp:connect(myIp,8888)
_:expect(result).is(nil)
tcp:close()
end)
_:test("Client connected to server", function()
local myIp = getMyIp()
local socket = require("socket")
local server = assert(getLocalServer())
server:settimeout(5)
local tcp = assert(socket.tcp())
tcp:settimeout(5)
local result = tcp:connect(myIp,8888)
_:expect(result, "couldn't connect").isnt(nil)
if result then
tcp:send("hello\n")
local serverconn, error = server:accept()
_:expect(serverconn, "serverconn").isnt(nil)
serverconn:settimeout(5)
local message = serverconn:receive()
_:expect(message, "server looks for hello").is("hello")
serverconn:send("goodbye\n")
line, error, partial = tcp:receive()
_:expect(line, "client looks for goodbye").is("goodbye")
end
server:close()
tcp:close()
end)
_:test("Can open two connections", function()
local ip = getMyIp()
local server = getLocalServer(0)
local socket = require("socket")
local master1 = assert(socket:tcp())
local ok1, error1 = master1:connect(ip,8888)
_:expect(ok1,"conn1 "..(error1 or "")).is(1)
local master2 = assert(socket:tcp())
local ok2,error2 = master2:connect(ip,8888)
_:expect(ok2, "conn2 "..(error2 or "")).is(1)
server:close()
master1:close()
master2:close()
end)
_:test("Server sees connect right after 'connect'", function()
local ip = getMyIp()
local server = getLocalServer(0)
local socket = require("socket")
local _nil, timeout = server:accept()
_:expect(timeout).is("timeout")
local master1 = assert(socket:tcp())
local ok1, error1 = master1:connect(ip,8888)
_:expect(ok1,"conn1 "..(error1 or "")).is(1)
local sConn1, error = server:accept()
_:expect(error).is(nil)
server:close()
master1:close()
end)
_:test("Two connections talking", function()
local ip = getMyIp()
local server = getLocalServer(0)
local socket = require("socket")
local master1 = assert(socket:tcp())
local ok1, error1 = master1:connect(ip,8888)
_:expect(ok1,"conn1 "..(error1 or "")).is(1)
local client1 = master1
local master2 = assert(socket:tcp())
local ok2,error2 = master2:connect(ip,8888)
_:expect(ok2, "conn2 "..(error2 or "")).is(1)
local client2 = master2
local serverC1,sError1 = server:accept()
_:expect(sError1).is(nil)
local serverC2,sError2 = server:accept()
_:expect(sError2).is(nil)
client1:send("I am c1\n")
local msg,err
msg,err = serverC1:receive()
_:expect(msg).is("I am c1")
_:expect(err).is(nil)
serverC1:send("hi c1\n")
local c1Msg, c1Err = client1:receive()
_:expect(c1Msg).is("hi c1")
_:expect(c1Err).is(nil)
client1:send("bye\n")
client2:send("never mind\n")
msg,err = serverC2:receive()
_:expect(err).is(nil)
_:expect(msg).is("never mind")
msg,err = serverC1:receive()
_:expect(err).is(nil)
_:expect(msg).is("bye")
serverC1:close()
serverC2:close()
server:close()
master1:close()
master2:close()
end)
end)
end
function getLocalServer(timeout)
local socket = require("socket")
local server = socket.bind(getMyIp(),8888)
if timeout then server:settimeout(timeout) end
return server
end
function getMyIp()
local socket = require("socket")
local s = socket.udp()
s:setpeername("1.1.1.1",80)
local myIp,_ignored = s:getsockname()
s:close()
return myIp
end