elixir-circuits / circuits_uart

Discover and use UARTs and serial ports in Elixir
Apache License 2.0
189 stars 48 forks source link

Issues around 4k limit handling. #61

Open SylwBar opened 5 years ago

SylwBar commented 5 years ago

Hi team. I'm evaluating Circuits.UART for my next project. We need to push a lot of usually small or sometimes bigger messages (50-2000 bytes) through serial line. Performance is critical aspect here - we want to send as many messages as possible. They could be glued together on serial line (separated by <<0x7E>> byte) After couple of days spent on testing Circuits.UART on different platforms I could summarize my observations in a list below. Issues 1-4 could be somehow mitigated by calling UART.drain but delay introduced with such approach (27ms on RasPI3) is not acceptable.

Could you evaluate how problematic will be to solve those issues? (if you agree on such classification) This is related to issue #26

Setup

Expected Behavior

  1. Sending messages longer that 16k should not cause circuits.uart port to close.
  2. Sending binaries longer than 4k should succeed, or: Sending binaries longer than 4k should return error information.
  3. User should be informed (possibly new function) about size of buffer limit as this limit could be platform dependent (?).
  4. Sending large number of messages (without waiting on drain) using UART.write should succeed assuming that single message is not bigger than 4k limit.
  5. Workaround for issue 2 is to call repeatedly UART.write and UART.drain with data blocks smaller than 4k, but there should be no gaps (ideally) in serial transmission visible.

Actual Behavior

  1. Sending messages longer that 16k prints: iex(1)> p = Bridge.test_start

    PID<0.159.0>

    iex(2)> Bridge.test_send p, 17000 circuits_uart: Message too long: 17026 bytes. Max is 16384 bytes (EXIT from #PID<0.157.0>) shell process exited with reason: an exception was raised: (ArgumentError) argument error :erlang.port_close(#Port<0.5140>) (stdlib) gen_server.erl:648: :gen_server.try_terminate/3 (stdlib) gen_server.erl:833: :gen_server.terminate/10 (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3

Interactive Elixir (1.7.4) - press Ctrl+C to exit (type h() ENTER for help) iex(1)> 13:00:56.033 [error] GenServer #PID<0.159.0> terminating ** (ArgumentError) argument error ... ...

  1. Logic analyzer attached to serial output shows that only first 4096 bytes are correctly sent, while the rest are zeroes. Function UART.write returns :ok in such case.
  2. 4k limit must be manually detected and hardcoded into main application (possibly not platform independent).
  3. Sending large number of messages (without waiting on drain) using UART.write will succeed only if summary of queued data length is smaller than 4k. Otherwise output data is garbage.
  4. Calling repeatedly UART.write and UART.drain with data blocks smaller than 4k, introduces delays between transfers: 23-29 miliseconds on RPI3, 10ms on 3.5GHz i7.

Steps to Reproduce the Problem

defmodule Bridge do
  alias Circuits.UART

  def test_start() do
    uart_name = "ttyS0"
    uart_speed = 115_200
    {:ok, uart_pid} = Circuits.UART.start_link()

    UART.open(uart_pid, uart_name,
      speed: uart_speed,
      active: false
    )

    uart_pid
  end

  def test_send(pid, len \\ 100, rep \\ 1, wait \\ false) do
    data = :binary.copy("1", len) <> <<0x7E>>

    Enum.each(1..rep, fn _ ->
      UART.write(pid, data) |> IO.inspect

      if wait do
        UART.drain(pid)
      end
    end)
  end
end

Issue 1: iex(1)> p = Bridge.test_start

PID<0.159.0>

iex(2)> Bridge.test_send p, 17000

Issue 2: iex(1)> p = Bridge.test_start

PID<0.159.0>

iex(2)> Bridge.test_send p, 4200

Issue 4: iex(1)> p = Bridge.test_start

PID<0.159.0>

iex(2)> Bridge.test_send p, 1500, 3

Issue 5: iex(1)> p = Bridge.test_start

PID<0.159.0>

iex(2)> Bridge.test_send p, 1500, 3, true

SylwBar commented 5 years ago

On issue #5: Could we add :drain parameter to UART.write to speed-up things?

fhunleth commented 5 years ago

Yes, completely agree. Thanks for the detailed write up.

Some issues, like the 16 KB one are not that hard to fix, buy you've already worked around that. Raw performance is something that I've wanted to spend time on, but I haven't had a need since the devices I use are very slow.

Unfortunately, unless something changes, I doubt that I'll be able to get to this issue this month. My guess is that I'll get to it too late to help you, so I really, really appreciate that you submitted a test program to help me reproduce the issue.

fhunleth commented 5 years ago

Regarding :drain parameter to UART.write, that seems like a reasonable thing to do. My intuition is that it won't help since the Elixir/port overhead will be masked by the time sending the bytes that will be drained. However, it's probably worth a simple test by adding a drain call in the C code after every write.

SylwBar commented 5 years ago

Your intuition was right. I added tcdrain(port->fd); at the end of uart_write() in uart_comm_unix.c and after call: Bridge.test_send p, 1500, 3 (now without call to UART.drain) I observe similar 20-30 ms gaps.

SylwBar commented 5 years ago

Hi. I figured out simple algorithm that helps me with pushing more messages on the link. In the previous example, I was waiting for the ending of each transfer using UART.drain.

Now I'm counting number of bytes that I already sent to UART buffer. When I'm sure that new data plus the one already send will fit in 4k then I'm not calling drain(). Otherwise I had to wait until transmission ends. There is corresponding logic. data_size is byte_size of new data. buf_len is sum of bytes I already sent.

new_buf_len =
        if buf_len + data_size > @max_buf do
          UART.drain(pid)
          data_size
        else
          buf_len + data_size
        end 

Finally I created new function test_send_wait() for testing this algorithm:

@max_buf 4090

  def test_send_wait(pid, len \\ 100, rep \\ 1) do
    data = :binary.copy("1", len) <> <<0x7E>>
    data_size = byte_size(data)

    Enum.reduce(1..rep, 0, fn _, buf_len ->
      new_buf_len =
        if buf_len + data_size > @max_buf do
          UART.drain(pid)
          data_size
        else
          buf_len + data_size
        end

      UART.write(pid, data)
      new_buf_len
    end)
  end 

This algorithm is very helpful in case of large number of small messages. In case of large messages (1000-2000 bytes) the improvement is clearly visible but the gaps are still there. Some screenshots. UART rate = 115_200 * 10. Without algorithm: wait with algorithm: wait_int

Transfer improvement is 80-100%. But bandwidth usage is appox. 60-70%.