| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Note that the default MTU forwarding policy is ONE_TO_ONE, therefore,
it is only strictly necessary to modify the MTU forwarding policy for
blocks that route data in a different manner. However, it may be nice to
explicitly state the forwarding policy for the benefit of the reader.
The following blocks had their policies updated:
- addsub: ONE_TO_FAN
- duc: ONE_TO_ONE
- dmafifo: ONE_TO_ONE
- null block: DROP
- replay block: DROP
- split stream: ONE_TO_FAN
- switchboard: ONE_TO_FAN
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously, the default was DROP. For almost all RFNoC blocks, this is
not a good default. It is very easy to crash USRPs by not properly
propagating the MTU. For example, the following flow graph:
Radio -> DDC -> FIR -> Streamer
would crash an X310 when not manually setting an spp value. The reason
is: The Radio block has an output buffer of 8192 bytes, capable of
handling 2044 samples per packet. However, that's too big for the
Ethernet portion of the X310, which would cause the X310 to lose
connection between UHD and firmware. If the FIR were configured to
propagate MTU, the Host->USRP connection (which has an MTU of <= 8000)
would limit the MTU on all links, and the spp value would automatically
be reduced to 1996 (or less).
This commit uses the post_init() feature to check the user set an MTU in
the constructor, and sets it to the default if that didn't happen. This
doesn't solve all problems (the new default of ONE_TO_ONE) could also be
incorrect, but is a much more suitable default.
As a consequence, this has a minor change in how
set_mtu_forwarding_policy() can be used: It now must be called during
the constructor. Before, the rule was that it may only be called once,
but that could also have happened, e.g., during the first property
resolution. Now, the constructor is the last time block authors can
choose an MTU forwarding policy.
|
| |
|
|
|
|
|
|
| |
This method allows running a fixed set of rules to check the internal
consistency of a block. This may be necessary, because blocks authors
may incorrectly implement a certain design rule, and we want the ability
to not start an RFNoC graph with blocks that have rule violations which
we can write checks for.
|
| |
|
|
|
|
|
|
|
|
| |
Constatntly incrementing endpoints was causing the entries in the
routing table on the device to be exhausted, eventually resulting in a
timeout error on control packets. Since a connection between the host
and a stream endpoint on a device in a given direction is unique, the
host endpoints can be cached and re-used. This change does that.
Signed-off-by: michael-west <michael.west@ettus.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In 0caed5529, a change was made to ctrlpoint_endpoint's behavior such
that if a client does not care about checking for ACKs on poke or poll
operations, the code calls `wait_for_ack()` with a flag indicating that
it should not wait for the ACK, but find and remove the corresponding
response from the response queue. This prevents the queue from
potentially growing endlessly with response packets that the client
doesn't even care about.
However, this introduced a subtle, undesired behavioral change. When
`wait_for_ack()` finds the corresponding response for a request, it also
checks the status field of the response to report any errors flagged by
the hardware such as invalid command, routing error, etc. Prior to the
change mentioned above, since `wait_for_ack()` was never called when the
client doesn't want ACKs, the client would never be never notified of
any errors associated with the request. However, with the aforementioned
change in placd, when `wait_for_ack()` is called to find and remove the
unwanted response packet corresponding to the request, errors **are**
checked and reported up the user.
The behavior change was unearthed by the X410 ZBX CPLD initialization
code, which writes an initial value of 0 to all ZBX CPLD registers--even
read-only registers. A control request to write a read-only register
results in a response with CMDERR in its status field, as it should.
However, since the ZBX CPLD register initialization is performed with a
`poke32()` operation which by default doesn't wait for ACKs, this was
never a problem until the change to drain the response queue
inadvertently caused the error to surface. The result is that creating a
USRP session or RFNoC graph session to an X410 device is seen to
occasionally fail with a 'RuntimeError: RuntimeError: Failure to create
rfnoc_graph' message printed to the console.
This commit preserves the desired queue-draining behavior, but ignores
any error status on the response when it is found and removed from the
queue, thus restoring the behavior pre-0caed5529.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This changes the behaviour of ctrlport_endpoint (the register interface
for block controllers) to always check for an ACK after doing a poke or
poll of any kind. Previously, the behaviour was to only check for an ACK
if the policy was set that way, or if the user requested the ACK to be
received.
The problem with the former approach was that if many pokes were
performed without ever requesting an ACK or a poll, the response queue
would fill up without ever getting emptied, eventually draining the
available heap space. Note that this is not a memory leak in the usual
sense, as the response queue was correctly holding on to the response
packets.
With this change, ctrlport_endpoint::wait_for_ack() now receives
a require_ack parameter. If it is false, the behaviour of wait_for_ack()
is changed as follows:
- If the response queue is empty, immediately return with an empty
response payload object.
- Otherwise, continue reading elements out of the response queue until
it is either depleted (in which case the previous rule kicks in), or
we find the ACK corresponding to the command previously sent out.
Note that this replicates the corresponding behaviour in UHD 3 (see
ctrl_iface_impl::wait_for_ack()).
|
| |
|
|
|
|
|
|
|
|
| |
The host code was calculating and programming a 32-bit value for the DSP
frequency, but the DDS modules in the FPGA only use the upper 24-bits.
This led to inaccurate frequency values being returned. This change
corrects the resolution of the value on the host side so an accurate
value is returned.
Signed-off-by: michael-west <michael.west@ettus.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
An RFNoC block (like the radio) might require a minimal number of
items in each clock cycle, e.g. the radio has to process
SPC (samples per cycle). Because data in RFNoC is transmitted and
processed in packets, we have to make sure the items inside these
packets are a multiple of the items processed in each cycle.
This commit adds an atomic item size properties which is set by
the radio and adapted by the streamers. The streamers adapt the
SPP property of the radio block controller depending on the MTU
value. This might lead to an SPP value which does not align with
the SPC value of the radio block, hence we add a property resolver
for the atomic item size.
|
| |
|
|
|
|
|
|
| |
As Github user johnwstanford points out, the DUC calls the argument
'input_rate', which is wrong (and was copy/pasted from the DDC code). By
calling it dds_rate in both cases, we avoid such confusion.
This commit only renames a variable. No changes whatsoever.
|
| |
|
|
|
| |
As Github user johnwstanford kindly points out, the comment was
incorrect.
|
| | |
|
| |
|
|
|
|
|
|
| |
Clarify that invalid RFNoC graph topology failures are due to an attempt
to access input or output ports that are not connected to anything in
the FPGA.
Signed-off-by: mattprost <matt.prost@ni.com>
|
| |
|
|
|
| |
The path it returned was only valid in UHD 3. Added unit test to
confirm.
|
| |
|
|
|
|
|
| |
In C++, variables whose address are taken must be defined somewhere.
PERIPH_BASE had no such definition, so on some compilers/systems caused
a linker error. This commit switches to using enums to prevent this
happening again in the future.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
On back-edges, no properties are forwarded, but properties must be
consistent after property resolution. This breaks when the source edge
on a back-edge has an edge property which the destination block does
not. Consider the following graph:
DDC -> Replay -> DDC
where both instances of 'DDC' refer to the same block. Now, assume the
first edge is declared a back edge (in principle, it shouldn't matter).
The DDC block has an edge property `samp_rate` which the Replay block
does not. Therefore, it can't forward this edge property to the Replay
block's input edge property list.
In the consistency check code, we don't check for the existence of edge
nodes, because it is assumed edge properties where either forwarded, or
aligned through some other manner. This leads to a property lookup
failure.
With this fix, we skip the consistency check for edge properties which
don't exist on the destination node. This is safe because the
destination block can not have a property resolver defined for undefined
properties. This means the destination block can either:
- Drop the property. In this case, there is no value in checking
consistency. Even if we could forward edge properties on back-edges,
they would always have the same value.
- Forward the property. In that case, the consistency check would happen
elsewhere in the graph where there's no back-edge.
|
| |
|
|
|
|
| |
The ops pending for each operation was stored implicitly in the data
structure. This adds it explicitly, which is useful for debugging
and packet dissection.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This class has a member _num_drops, which can be read out using the
get_num_drops() API call. However, when dropping packets, this counter
was not incremented, which is fixed now.
This also includes a very minor optimization from 2 map<> lookups to
1 lookup (they are in O(log N)). Since there are usually a small
two-digit number of endpoints connected to the async message receiver,
this change is not expected to yield major improvements, but the lookup
*is* in a hot loop.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The default block controller is used whenever no other block controller
is used. It currently defaults to dropping both property propagation and
actions.
When a custom block is injected into a graph like this for example:
Radio -> DDC -> Custom Block -> Rx Streamer
This default behaviour causes the Rx Streamer to not be able to send
actions (like stream commands) nor does it allow MTU propagation (or any
other property's propagation).
The default block behaviour is ONE_TO_ONE, meaning that actions and
properties on input channel N will get forwarded to output channel N. In
absence of an actual block controller, this is more useful default than
setting the propagation to DROP for both actions and properties. Most
blocks that pass through data, or do some simple processing, will now
work in the absence of a block controller.
The new disadvantage is that blocks which would modify properties such as
sampling rate, scaling, or MTU will no longer work properly in the
absence of a block controller.
However, the recommended behaviour is anyway not to operate without a
block controller. For the cases where no block controller is present,
ONE_TO_ONE is considered the generally more useful default.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
These two values where being mixed up in the code. To summarize:
- The MTU is the max CHDR packet size, including header & timestamp.
- The max payload is the total number of bytes regular payload plus
metadata that can be fit into into a CHDR packet. It is strictly
smaller than the MTU. For example, for 64-bit CHDR widths, if
a timestamp is desired, the max payload is 16 bytes smaller than
the MTU.
The other issue was that we were using a magic constant (DEFAULT_SPP)
which was causing conflicts with MTUs and max payloads.
This constant was harmful in multiple ways:
- The explanatory comment was incorrect (it stated it would cap packets
to 1500 bytes, which it didn't)
- It imposed random, hardcoded values that interfered with an 'spp
discovery', i.e., the ability to derive a good spp value from MTUs
- The current value capped packet sizes to 8000 bytes CHDR packets, even
when we wanted to use bigger ones
This patch changes the following:
- noc_block_base now has improved docs for MTU, and additional APIs
(get_max_payload_size(), get_chdr_hdr_len()) which return the
current payload size given MTU and CHDR width, and the CHDR header
length.
- The internally used graph nodes for TX and RX streamers also get
equipped with the same new two API calls.
- The radio, siggen, and replay block all where doing different
calculations for their spp/ipp values. Now, they all use the max
payload value to calculate spp/ipp. Unit tests where adapted
accordingly. Usage of DEFAULT_SPP was removed.
- The replay block used a hardcoded 16 bytes for header lengths, which
was replaced by get_chdr_hdr_len()
- The TX and RX streamers where discarding the MTU value and using the
max payload size as the MTU, which then propagated throughout the
graph. Now, both values are stored and can be used where appropriate.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
The replay block is more like the radio block than like a FIFO. In
particular, consider this flow graph:
Replay -> DDC -> Replay
Imagine you're using the replay block to test the DDC block with
prerecorded data. If we treated the Replay Block like a FIFO, then we'd
have a loop in the graph (which is already wrong). If we used the DDC to
resample, then the input- and output sample rate of the Replay mismatch,
which is a legal way to use the Replay block, but not possible if we
treat the graph like a loop.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The async message handler and the async message validator would
erroneously compare channel numbers for RX async messages with the
number of valid TX channels. On TwinRX, where there are zero TX
channels, this would always fail. Elsewhere in the code, the comparisons
for TX and RX channels mixed up input and output ports.
The second issue is that the comparison made was a "greater than" rather
than "greater or equal".
The effect of these two bugs was that potentially, we could have
accepted async messages for an invalid port N, where N is the number of
valid ports of this block, and that for TwinRX/X300 users, async
messages on channel 1 would not get accepted (they would, however, get
accepted for channel 0 because of the second issue). This includes
overrun handling, which was broken for channel 1 and 3 on an X300.
Another effect of the bug was that EPIDs for async messages weren't
always programmed correctly.
|
| |
|
|
|
|
|
| |
Getting the time from the mb_controller is slow, so try to get the time
from the Radio on the fast path first.
Signed-off-by: michael-west <michael.west@ettus.com>
|
| |
|
|
|
|
| |
Add API calls to Radio control to get ticks and time.
Signed-off-by: michael-west <michael.west@ettus.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
The order must:
- Check transaction has the right number of hops, then read hop
- Check hop has the right number of operations (at least 2), then read
those ops
- Check the ops have the correct opcodes
The code was doing checks in the wrong order. Thanks to Github user
johnwstanford for pointing this out.
|
| |
|
|
|
|
|
| |
This provides every block controller with a copy of its CHDR width.
Note: mock blocks always get configured with a 64-bit CHDR width, to
retain API compatibility.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
This removes some constants from UHD that were left over from RFNoC/UHD
3.x. They are unused.
rfnoc_rx_to_file had a commented-out section that was also UHD-3 only.
Note that rfnoc/constants.hpp is pretty bare now, and could be removed.
However, it is in the public header section, so we shall leave the used
constants where they are.
This requires fixing includes in mgmt_portal.cpp.
|
| |
|
|
|
|
|
|
|
| |
The I and Q were swapped in sine_tone, which caused confusion and made
the rotation of REG_CARTESIAN clockwise by default. This effectively
made the resulting frequency negative. This PR makes the I and Q order
consistent with RFNoC and fixes the direction of rotation so that a
positive value for REG_PHASE_INC (phase increment) results in a
counter-clockwise rotation, which yields a positive frequency.
|
| |
|
|
| |
Thanks for github user johnwstanford for pointing those out.
|
| |
|
|
|
|
|
|
|
| |
The variable max_size_bytes has a different name in the source than in
the header and is not self-explanatory in both. Therefore when comparing
against it in the assertion in line 142 one could assume that a number
of bytes needs to be compared with a byte value. Change variable to
`buff_size` in source and header file to avoid confusion and add
documentation.
|
| |
|
|
|
| |
This modifies some log messages or exception strings when using
auto-correction APIs that are not supported by the underlying device.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The tuning range of the DUC depends on the output sample rate (which is
larger), but it was using the input sample rate. This was causing a bug
where for Tx, the DSP tuning range was limited when using multi_usrp API,
and thus would not allow to DSP-tune beyond the current sampling rate.
In this patch, we also re-use the existing calculation of the sampling
rate, and harmonize that code between duc_block_control and
ddc_block_control.
Consider the following Python REPL code:
>>> import uhd
>>> U = uhd.usrp.MultiUSRP('type=x300')
>>> U.set_rx_rate(10e6)
>>> U.set_tx_rate(10e6)
>>> # Creating a streaming is required, or the input rate will not be
>>> # set:
>>> S = U.get_tx_stream(uhd.usrp.StreamArgs("fc32", "sc16"))
>>> treq = uhd.types.TuneRequest(1e9)
>>> treq.rf_freq = 1e9
>>> treq.dsp_freq = 50e6
>>> treq.dsp_freq_policy = uhd.types.TuneRequestPolicy.manual
>>> treq.rf_freq_policy = uhd.types.TuneRequestPolicy.manual
>>> tres = U.set_rx_freq(treq, 0)
>>> print(str(tres))
Tune Result:
Target RF Freq: 1000.000000 (MHz)
Actual RF Freq: 1000.000000 (MHz)
Target DSP Freq: 50.000000 (MHz)
Actual DSP Freq: 5.000000 (MHz)
>>> # Note the last two lines: The *target* DSP freq was already clipped
>>> # to 5 MHz. These lines show 50.0 MHz when this patch is applied.
This bugfix is accompanied some related changes:
- The unit test is amended to verify the behaviour
- The API documentation is amended with details on its behaviour
|
| |
|
|
|
| |
This commit adds `get_src_epid()` and `get_port_num()` method bindings
to the Python bindings for `noc_block_base`.
|
| |
|
|
|
|
| |
Fix implicit typecasts that could potentially lose data. Doing this to
show that these typecasts are done on purpose (and to resolve warnings
from VS).
|
| |
|
|
|
|
| |
Fix the "Enum.3: Prefer class enums over "plain" enums" warning for the
node_type enum and update the calls to the enumerators as proposed by
the C++ Core Guidelines.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
By changing the type for accesses to noc_block_base calls in the Python
from sptr& to a simple reference (&), we fix the "holder type" issues
that crop up when trying to use radio_control from multi_usrp, which
returns access to the block as a reference rather than a `sptr`.
The error message seen without this fix always contains this string:
Unable to cast from non-held to held instance (T& to Holder<T>)
(The exact message depends on the API call made).
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
In current implementation it is not possible to find all blocks of
a device by calling find_blocks("0/"). The same is true for the
block count. This is caused by the valid block id regex which
requires a block name. This regex is used to validate the block
name as well as to match block ids in search.
This fix looses the requirement for the block name to allow
searches by device number and block count and also extends the
is_valid_block_id method to require the block name match to be
non empty (which restores the previous behaviour at this point).
|
| |
|
|
|
|
|
| |
meta_range_t(0,0) actually calls the iterator-based constructor for
meta_range_t, which is almost certainly not the intended constructor
for that call syntax. Therefore, we add a static_assert to prevent
such usage, and fix all failing instances.
|
| |
|
|
|
|
|
|
| |
The Boost version is identical to the std:: version (which is available
since C++11) and thus is no longer needed.
Because of implicit includes, this breaks compilation in other parts.
Appropriate includes were added there also.
|
| |
|
|
|
|
|
| |
Its behaviour is almost identical to std::lround, which we use instead.
The only downside of std::lround is that it always returns a long, which
we don't always need. We thus add some casts for those cases to make the
compiler happy.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Co-authored-by: Lars Amsel <lars.amsel@ni.com>
Co-authored-by: Michael Auchter <michael.auchter@ni.com>
Co-authored-by: Martin Braun <martin.braun@ettus.com>
Co-authored-by: Paul Butler <paul.butler@ni.com>
Co-authored-by: Cristina Fuentes <cristina.fuentes-curiel@ni.com>
Co-authored-by: Humberto Jimenez <humberto.jimenez@ni.com>
Co-authored-by: Virendra Kakade <virendra.kakade@ni.com>
Co-authored-by: Lane Kolbly <lane.kolbly@ni.com>
Co-authored-by: Max Köhler <max.koehler@ni.com>
Co-authored-by: Andrew Lynch <andrew.lynch@ni.com>
Co-authored-by: Grant Meyerhoff <grant.meyerhoff@ni.com>
Co-authored-by: Ciro Nishiguchi <ciro.nishiguchi@ni.com>
Co-authored-by: Thomas Vogel <thomas.vogel@ni.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
When a node has an action callback assigned this must be cleared
along with the block removal. Otherwise a post action callback
might try to modify node that are already removed which results
in an undefined behavior.
In particular this one fixes the
Unexpected error [ERROR] [CTRLEP] Caught exception during async message handling: map::at
when running the multi_usrp_test.py
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In 073574e24, the MTU property resolver in `noc_block_base` was
refactored to make the resolver's output sensitivity list less
broad. The broadness was intentional as a consequence of allowing the
MTU forwarding policy to be changed at will, but had the unintended side
effect of being incompatible with certain RFNoC graph use cases. The
refactoring solved the issues, but added a new restriction that the MTU
forwarding policy could only be called once per instance of a NoC block.
Unfortunately, that refactoring introduced a bug. By moving the
registration of MTU resolvers to `set_mtu_forwarding_policy()`, no
resolvers would be added if the MTU forwarding policy was never changed
from the default of `DROP` (which is the case for the vast majority of
NoC blocks). However, the resolver had code that would run in the `DROP`
case to coerce the incoming MTU edge property to be the smaller of the
new value or the existing MTU value on that edge. With the resolvers
only getting added when the MTU forwarding policy is changed, this
coercion behavior would never execute, thus breaking a number of
devtests.
This commit ensures that the default coercion behavior is always present
regardless of whether the MTU forwarding policy is changed or not.
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Prior to this commit, the MTU property resolver in noc_block_base had an
issue: for every MTU edge property (both input and output on each port)
on the block, the property resolver listed every other MTU edge property
in its output sensitivity list, regardless of whether or not the output
edge properties would ever be affected by the current MTU forwarding
policy. This breaks an inherent (and up until now, unwritten) contract
between a property resolver and UHD that only properties that can be
affected by the resolver should be included in the output sensitivity
list. The result of breaking the contract leads to errors being thrown
when committing an RFNoC graph in certain multi-channel use cases.
This commit refactors the MTU property resolver to use the MTU
forwarding policy to determine the correct set of edge properties to
include in the output sensitivity list. The change also introduces a new
restriction--the MTU forwarding policy may only be set once per instance
of a noc_block_base. Typically, a subclass implementing an RFNoC block
will call `set_mtu_forwarding_policy()` in its constructor to set a
custom MTU forwarding policy (if desired) and leave it untouched for the
lifetime of the block.
|
| |
|
|
|
|
|
|
|
| |
A loop in mgmt_portal::_validate_stream_setup() was missing a sleep,
which was causing it to return long before the timeout with a timeout
error. This change adds that sleep and reduces the duration of the
sleep so it responds faster.
Signed-off-by: Michael West <michael.west@ettus.com>
|
