search

sudomesh / LoRaLayer2

Layer 2 routing protocol for LoRa connected devices
86 stars 29 forks source link

Revert sending outgoing messages back to the incoming buffer #2

Closed tlrobinson closed 4 years ago

tlrobinson commented 4 years ago

I noticed this commit in LoRaLayer2 https://github.com/sudomesh/LoRaLayer2/commit/2f6ecd1b0f8d97aae9d8bc964a12b9f452d18c7f#diff-990b5ac4edb2f90f76a2d731666dc92cR621 adds logic to broadcast outgoing messages back to the incoming message buffer, which feels wrong to me. I think it should be up to the consumers of the library to decide when those messages should be echoed back.

For example, my refactor of disaster-radio broadcasts messages to all clients (except the sending client) already: https://github.com/sudomesh/disaster-radio/pull/44/files#diff-ea8c5481dd4f3db1633095cbd97be1aeR25-R34 This logic would cause all outgoing messages to be duplicated on other clients.

paidforby commented 4 years ago

You are not exactly correct. There is only one particular type of outgoing packet that is being sent to the chat/WS buffer, if you look at the whole piece of code,

struct Packet packet = buildRoutingPacket();
pushToOutBuffer(packet);
pushToWSBuffer(packet);

The only outgoing packet being sent is a routing packet. This is essentially a "hack" to get the current routing table to web app so it can be displayed below the chat. So really, this should be thought of as a "meta-data" message from LL2 to the WS (via the WS-bound buffer), rather than an outgoing packet being sent to an incoming packet buffer. The commit you mention was directly related to this issue in the disaster-radio repo, https://github.com/sudomesh/disaster-radio/issues/35#issuecomment-574992472

I knew this was a bad hack, but I was more focused on getting the web app side of the code working.

Ideally, I want the LL2 code to have four buffers.

  1. LL2 -> Layer1 (outgoing packets from either LL2 or WS)
  2. LL2 <- Layer1 (incoming packets from Layer1)
  3. LL2 -> WS (incoming messages from Layer1 or metadata from LL2)
  4. LL2 <- WS (outgoing message from WS)

Currently, only the first three buffers exist, because I haven't had a really good reason to implement the fourth one yet. Notice in the third buffer it can be used for any information needed by the WS.

Maybe this kind of setup only makes sense to me. Or maybe there should be a different buffer for informative meta-data between LL2 and WS. I haven't had anyone looking at my code and reviewing it, so I've just been doing whatever makes sense at the time, it's good to have someone else's input. I haven't had a chance to dig into your PR. I will comment in that thread once I look over it.

tlrobinson commented 4 years ago

Sorry, you are correct. I assumed this change was to fix an issue I saw previously about sending outgoing messages to other WS clients on the same node.