JuliaComputing / xtrx_julia

XTRX LiteX/LitePCIe based design for Julia Computing
BSD 2-Clause "Simplified" License
1 stars 0 forks source link

Fixes and improvements towards support for high-level API #59

Closed sjkelly closed 2 years ago

sjkelly commented 2 years ago

This reworks the test_pattern script to use the high level API. This (unsurprisingly) fails, as the interlacing code needs to be finished. It was copied from HackRF, but it was only robustly testable now in this script. I modified the assertions assuming the following:

The basic DMA format is:

[(Ia_1, Ib_1), (Qa_1, Qb_1), (Ia_2, Ib_2) ...] 

So both the I values for Channel A and B are sent together, followed by the Q.

We want to transform this into one array for each channel of Complex Int16. Ideally we also sign extend and such so the simplest API in Soapy may be used without any device specific hacks.

maleadt commented 2 years ago

I think there will be more required than just some deinterlacing -- much of the functionality for this is entirely untested, and there seem to be bugs lurking (both in SoapySDR.jl and in the XTRX streaming functionality).

I'm using this loopback test script:

ENV["SOAPY_SDR_LOG_LEVEL"] = "DEBUG"

using SoapySDR, Printf

#SoapySDR.register_log_handler()

function loopback_test(num_channels=1)
    # open the first device
    devs = Devices(parse(KWArgs, "driver=XTRX"))
    dev = Device(devs[1])

    # get the RX and TX channels
    chan_rx = SoapySDR.Channel[]
    chan_tx = SoapySDR.Channel[]
    for i in 1:num_channels
        push!(chan_rx, dev.rx[i])
        push!(chan_tx, dev.tx[i])
    end

    # enable a loopback
    SoapySDR.SoapySDRDevice_writeSetting(dev, "LOOPBACK_ENABLE", "TRUE")

    # open RX and TX streams
    format = Complex{Int16} #chan_rx.native_stream_format
    stream_rx = SoapySDR.Stream(format, chan_rx)
    stream_tx = SoapySDR.Stream(format, chan_tx)

    # properties that matter for the buffers we'll be using
    mtu = Int(stream_tx.mtu)

    try
        wr_cnt = 0
        rd_cnt = 0
        should_transmit() = wr_cnt < 999
        should_receive() = rd_cnt < 3000
        # NOTE: we read more buffers in case there's delay between TX and RX

        SoapySDR.activate!(stream_rx)
        SoapySDR.activate!(stream_tx)

        while should_transmit() || should_receive()
            if should_transmit()
                buffs = []
                for i in 1:num_channels
                    buf = Vector{format}(undef, mtu)
                    buf .= (wr_cnt += 1)
                    push!(buffs, buf)
                end
                write(stream_tx, tuple(buffs...))
            end

            if should_receive()
                buffs = []
                for i in 1:num_channels
                    buf = Vector{format}(undef, mtu)
                    push!(buffs, buf)
                end
                read!(stream_rx, tuple(buffs...))

                if rd_cnt % 32 == 0
                    println()
                end
                @printf("%3d ", real(buffs[1][1]))
            end
        end
        println()
    finally
        SoapySDR.deactivate!(stream_tx)
        SoapySDR.deactivate!(stream_rx)

        # close everything
        finalize.([stream_rx, stream_tx])
        finalize(dev)
    end
end

isinteractive() || loopback_test()
maleadt commented 2 years ago

Found another remaining issue (but don't have the time to fix it right now, so putting it out here in case somebody does): in SoapySDR.jl's high-level wrappers, SoapySDRDevice_readStream is called and the result unconditionally added to a counter. However, it isn't checked whether the return value was a valid MTU, or a (negative) error code. In our case, due to initial compile-time latency, the internal buffers quickly overflow and the first couple of read calls return an -OVERFLOW error code. That results in a negative counter, causing us to acquire lots of buffers and ultimately trigger a timeout.

So this is a bug in SoapySDR, but we might also have to rethink the XTRX's streaming functionality: If an overflow occurs, we mark the buffer as acquired (by having bumped user_count), and assume the user will retire the buffer (which will bump sw_count and start to catch up with the hardware). I'm not sure what the semantics of SoapySDR errors are -- should the user call releaseBuffer if acquireBuffer returned an error? If so, then SoapySDR.jl's high-level wrappers are wrong. Or is this what the SOAPY_SDR_END_ABRUPT flag is for?

staticfloat commented 2 years ago

I think this is sufficiently subtle that we're going to need to discuss it together on a call. Thanks for investigating though!

maleadt commented 2 years ago

I've kind-of hijacked this PR -- sorry @sjkelly -- but since it contains some actual fixes that surface in other cases as well I propose merging this and working on the actual high-level API in a follow-up PR. To that end, I removed the PR that switched test_pattern.jl to the high-level API, so only fixes remain.

OK with you @sjkelly?