ShadowKatStudios / OC-Minitel

Easy-to-implement networking protocol for OpenComputers
https://oc.shadowkat.net/minitel/
Mozilla Public License 2.0
41 stars 12 forks source link

Minor tweaks to OpenOS/etc/rc.d/minitel.lua to improve performance #39

Open Jajasek opened 3 years ago

Jajasek commented 3 years ago
  1. When a server is repeating a packet, the destination is cached in rcache and rcache[dest][1] is the same modem the packet arrived through, the packet should not be repeated. This could be solved by adding a condition in the function sendPacket (on line 119).
  2. The comment on line 37 suggests the time in rcache should be updated when a new packet from that computer is received, but it is not implemented. The function processPacket writes to rcache only if the computer is not cached. The condition on line 189 should be removed, the line rcache[sender] = {localModem,from,computer.uptime()+cfg.rctime} should be executed for all packets. Therefore when 2 computers periodically communicate, the servers between them never forgot the route. Also, it should be executed before the ack packet is sent. I therefore suggest to move it before the line 172.
  3. Cosmetic issue: the packets contain the vPort entry, but it is not documented in the comment on line 10.
Jajasek commented 3 years ago

I have done some more playtesting and revealed another 2, much more severe, mistakes.

  1. In file OpenOS/usr/lib/minitel.lua, function net.rsend() returns true if any ack packet is seen, not only the one with rpid == pid.
  2. The broadcast messages are totally broken. I wonder how could this error persist here for so long time. When the function processPacket() encounters a broadcast message, it pushes net_broadcast signal, but the arrangement of the conditionals prevent it from routing the packet. I fixed it and this is my solution (it fixes even problem 2 mentioned above):
    local function processPacket(_,localModem,from,pport,_,packetID,packetType,dest,sender,vPort,data,...)
    pruneCache()
    if pport == cfg.port or pport == 0 then -- for linked cards
    dprint(cfg.port,vPort,packetType,dest)
    if checkPCache(packetID) then return end
    dprint("rcache: "..sender..":", localModem,from,computer.uptime())
    rcache[sender] = {localModem,from,computer.uptime()+cfg.rctime}
    if dest:sub(1,1) == "~" then -- broadcasts start with ~
      computer.pushSignal("net_broadcast",sender,vPort,data)
    end
    if dest == hostname then
      if packetType == 1 then
        sendPacket(genPacketID(),2,sender,hostname,vPort,packetID)
      end
      if packetType == 2 then
        dprint("Dropping "..data.." from queue")
        pqueue[data] = nil
        computer.pushSignal("net_ack",data)
      end
      if packetType ~= 2 then
        computer.pushSignal("net_msg",sender,vPort,data)
      end
    elseif cfg.route then -- repeat packets if route is enabled
      sendPacket(packetID,packetType,dest,sender,vPort,data,localModem)
    end
    if not pcache[packetID] then -- add the packet ID to the pcache
      pcache[packetID] = computer.uptime()+cfg.pctime
    end
    end
    end
Leonard2 commented 3 years ago

Thanks a lot for the fix!

Kurtoid commented 2 years ago

Can you add this as a pull request?