grc: refactor c++ generation and fix zero-port busses #7072
Conversation
|
Seems like too much to put into 3.10.9.2. Opinions? |
Ha! I might be biased, and would opine otherwise 🗡️ But let's look at that in detail: Commit grc: fix C++ gen with busports fixes the C++ generation issue and completes #7067 Commit grc: minor refactoring of cpp_top_block is but minor mechanical refactorings, which don't need to be backported, but would make Commit grc: fix busport creation with 0 ports is a bit "raw" in that it combines a bugfix (which should be backported, but isn't very high prio) with refactoring. That's mostly due to me having to do the refactoring to be able to fix the bug. I can look into extracting the quality-of-life improvements out of that commit (that is, mainly, consistent string quoting/f-strings), but it'd be sad to also remove the added logging, which was helpful knowing where things went wrong. The actual refactoring was rather benign, in essence: extract a function and convert def handle_ports(self, ports):
for thing in {"foo", "bar"}:
if thing == "foo":
baz = self.a[thing]
elif thing == "bar":
baz = self.b[thing]
a_lot_of_code_here_using_ports_and_bazto def do_thing(self, baz):
a_lot_of_code_here_using_ports_and_baz
def handle_ports(self, ports):
if not ports:
return_early
self.do_thing(ports, self.a["foo"])
self.do_thing(ports, self.b["bar"]) |
|
I'll give it a closer look. I was hoping we could keep the 3.10.9.2 changes down to really small things that are easily checked in a diff. |
|
Amended last commit with python formatting, cleaned up commit message. |
| @@ -13,7 +10,7 @@ | |||
|
|
|||
| DATA_DIR = os.path.dirname(__file__) | |||
|
|
|||
| PYTHON_TEMPLATE = os.path.join(DATA_DIR, 'flow_graph.py.mako') | |||
|
Can you point out the bugfix for 0-length bus ports? The ' to " conversion makes this diff hard to read. |
| @@ -492,10 +506,10 @@ def by_domain_and_blocks(c): | |||
| for port_num in porta.bus_structure: | |||
| hidden_porta = porta.parent.sources[port_num] | |||
| hidden_portb = portb.parent.sinks[port_num] | |||
| connection = fg.parent_platform.Connection( | |||
There was a problem hiding this comment.
This is the line equivalent to the Python fix.
There was a problem hiding this comment.
Yeah, I found that -- and it's also nicely put into its own commit. It seemed like there's another bugfix here regarding 0-length bus ports...? What's even the bug?
| if not ports: | ||
| self.current_bus_structure[direc] = None | ||
| return |
There was a problem hiding this comment.
This is the core fix, @mbr0wn
sure, @mbr0wn , #7072 (comment) ; but find that in the "from" code… |
- clean up includes - active block filtering refactor - variable type mapper refactor - param type conv refactor Signed-off-by: Marcus Müller <mmueller@gnuradio.org>
needed refactoring: short circuit zero port behavior, use f-strings, actually handle Exceptions, log things, consistent quotation marks, sorted stdlib includes… Signed-off-by: Marcus Müller <mmueller@gnuradio.org>
willcode
changed the title
|
Split simple fix commit out to another PR for quick merge. We need to back out the unneeded |
|
Do portions of this still apply after the related PR that was merged? |
|
@willcode good question, don't have resources right now to look at that in detail
You could right-click on a block with no output ports, and select "create bus source port", and it would suddenly get a (very confusing, and dysfunctional) white bus output port, combining all 0 ports into 1 bus port. (same for sink) |
Description
Busports flowgraphs couldn't be generated. Martin fixed the Python case, this fixes the C++ case.
As a drive-by, this untangles a bit of the block.py code, which was necessary for me to actually understand it.
Related Issue
Fixes #7069
Which blocks/areas does this affect?
GRC
Testing Done
made https://gist.github.com/marcusmueller/5cd33ac2f6cf752049ddf7a50b7bc974 compile
Checklist