Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rfnoc_modtool create and add requires some work after #813

Open
zacaric opened this issue Nov 20, 2024 · 6 comments
Open

rfnoc_modtool create and add requires some work after #813

zacaric opened this issue Nov 20, 2024 · 6 comments
Labels

Comments

@zacaric
Copy link

zacaric commented Nov 20, 2024

Issue Description

I am using rfnoc_modtool to create an oot module called core and a block called square (squares the IQ data).

When doing this, there are a few issues that causes things to not work right away and requires changes right out of the box (See Actual Behavior to see what changes are required to get working). Some of the items in the list I can understand needing to do, but other items I feel should have been handled by rfnoc_modtool when generating the files.

Setup Details

UHD 4.7.0.0-149-g635ad362, Ubuntu 22.04, Vivado 2021.2 AR76780

Expected Behavior

Expected for files generated from rfnoc_modtool to work out of the box with little modification. Also expected adding an icore file would also work out of the box. Little modification is understandable, but some documentation on that would be helpful.

Actual Behavior

The following files require modification:

  • rfnoc-core/CMakeLists.txt needs find_package (Python3 COMPONENTS Interpreter Development) added (I added it after line 43 and it worked)
    • (Note mbr0wn: This is actually a bug in UHDPython.cmake, adding this line may be required some systems, not on others, but we will include a fix in UHD itself instead of adding this here).
  • rfnoc-core/lib/square_block_control.cpp change "Gain" to "Square" on line 30
  • rfnoc-core/python/square_block_control_python.hpp needs to have line 12 changed from using namespace rfnoc::gain; to using namespace rfnoc::core;
  • rfnoc-core/fpga/core/CMakeLists.txt needs add_subdirectory(rfnoc_block_square) added
  • Create the file rfnoc-core/fpga/core/rfnoc_block_square/CMakeLists.txt and add the following:
    #
    # Copyright 2019 Ettus Research, a National Instruments Brand
    #
    # SPDX-License-Identifier: GPL-3.0-or-later
    #
    
    # This macro will tell CMake that this directory contains an RFNoC block. It
    # will parse Makefile.srcs to see which files need to be installed, and it will
    # register a testbench target for this directory.
    RFNOC_REGISTER_BLOCK_DIR()
    
    # This will do the same, but it will skip the testbench target.
    #RFNOC_REGISTER_BLOCK_DIR(NOTESTBENCH)
  • rfnoc-core/fpga/core/rfnoc_block_square/Makefile.srcs needs the file extenstions to be change from *.v to *.sv
  • rfnoc-core/fpga/core/rfnoc_block_square/rfnoc_block_square.sv needs a comma at the end of line 182
  • rfnoc-core/fpga/core/rfnoc_block_square/rfnoc_block_square.sv needs quotes around HDL_IP on line 26
  • rfnoc-core/fpga/core/rfnoc_block_square/noc_shell_square.sv needs a comma at the end of line 116
  • rfnoc-core/fpga/core/rfnoc_block_square/noc_shell_square.sv needs quotes around HDL_IP on line 36
  • rfnoc-core/icores/CMakeLists.txt requires RFNOC_REGISTER_BLOCK to be changed to RFNOC_REGISTER_IMAGE_CORE

Even after doing this, I am still getting the following error: UPDATE: Found out the following error had to do with an incorrect path in my square.yml file and not the rfnoc_modtool

⛔   Error evaluating square0: File fpga/square/rfnoc_block_square/Makefile.srcs not found in include paths: ['/workspaces/rfnoc-project-example/src/oot/rfnoc-core', '/usr/local/share/uhd/rfnoc']
⛔   Image configuration contains issues: Skipping build. Use --ignore-warnings to build despite warnings.

Steps to reproduce the problem

rfnoc_modtool create core
cd rfnoc-core
rfnoc_modtool add square

# Next, do these steps
# 1. copy x310_rfnoc_image_core.yml from the gain example
# 2. change any instance of "gain" to "square"

mkdir build
cd build
cmake -DUHD_FPGA_DIR=/uhd/fpga ../
make # This fails require files to be modified
make install
make x310_rfnoc_image_core # This fails requiring even more files to be modified

Additional Information

I am running all of this in a docker image (using Ubuntu 22.04 as the base). I was able to get the rfnoc_gain example working in this docker image.

@mbr0wn
Copy link
Contributor

mbr0wn commented Nov 26, 2024

@zacaric Thanks for this super helpful report. FYI, we generate the OOT sources from rfnoc-gain, so that explains some of the issues, but not all of them. We're investigating and will provide a fix soon.

@mbr0wn mbr0wn added the bug label Nov 26, 2024
@zacaric
Copy link
Author

zacaric commented Nov 26, 2024

Another modification I had to make afterwards was to add a comma on line 182 of rfnoc-core/fpga/core/rfnoc_block_square/rfnoc_block_square.sv. Same thing with rfnoc-core/fpga/core/rfnoc_block_square/noc_shell_square.sv on line 116.

I will add this to the description as well to keep these needed modifications in one place

@mbr0wn
Copy link
Contributor

mbr0wn commented Nov 26, 2024

@zacaric do you have f0066cf? That should take care of the noc_shell fixes.

@zacaric
Copy link
Author

zacaric commented Nov 26, 2024

It seems like I do have those changes. I have my docker image setup to grab the latest code on the master branch every time it gets built and I rebuilt the image yesterday. So it should be running to most up to date version on master (UHD 4.7.0.0-149-g635ad362).

I also found some errors with double quotes (") not being around HDL_IP in a couple of the files. I added that to the issue description too.

ip_option I think should also be changed to IP_OPTION in those files since that is what it is in the Gain example, but it doesn't really break anything so I am not adding it in the description.

I also added that rfnoc-core/lib/square_block_control.cpp needs to be changed from "Gain" to "Square" on line 30

@mbr0wn
Copy link
Contributor

mbr0wn commented Nov 28, 2024

@zacaric I hope you don't if I edit your list into a checklist so I can keep track of things that I've locally fixed. Actually scratch that, I'll keep track of it locally.

@mbr0wn
Copy link
Contributor

mbr0wn commented Nov 28, 2024

@zacaric FYI we have a patchset incoming with fixes to all the issues. It's going through an internal review process right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants