Rewrite of Exporter, Minor Changes in Importer by micha91 · Pull Request #15 · dbinfrago/capella-ros-tools

micha91 · 2025-09-17T09:51:36Z

No description provided.

# Conflicts: # capella_ros_tools/importer.py

capella_ros_tools/export_templates/cmake_pkg_level.j2

capella_ros_tools/exporter.py

README.md

Co-authored-by: Martin Lehmann <martin.lehmann@deutschebahn.com>

Wuestengecko

Yeah IDK if it works then I'm okay with it I guess. Just some minor notes.

Wuestengecko · 2025-12-05T10:35:42Z

capella_ros_tools/__main__.py

+    help="Decide whether experimental cmake files should be generated.",
+    envvar="CAPELLA_ROS_TOOLS_GENERATE_CMAKE",
+)
+def export(


Now that the function is called export (just like the click command), we no longer need to pass the command name explicitly in the outermost decorator.

-@cli.command("export") +@cli.command()

Wuestengecko · 2025-12-05T10:40:08Z

capella_ros_tools/__main__.py

+@click.option(
+    "--generate-cmake",
+    type=bool,
+    default=False,
+    is_flag=True,
+    help="Decide whether experimental cmake files should be generated.",
+    envvar="CAPELLA_ROS_TOOLS_GENERATE_CMAKE",
+)


I'm a fan of short flag names. And short help texts. :)

Suggested change

@click.option(

"--generate-cmake",

type=bool,

default=False,

is_flag=True,

help="Decide whether experimental cmake files should be generated.",

envvar="CAPELLA_ROS_TOOLS_GENERATE_CMAKE",

)

@click.option(

"--cmake",

type=bool,

default=False,

is_flag=True,

help="Generate cmake files. Experimental.",

envvar="CAPELLA_ROS_TOOLS_GENERATE_CMAKE",

)

Wuestengecko · 2025-12-05T10:52:42Z

capella_ros_tools/__main__.py

+            raise RuntimeError(
+                "Neither config nor root package nor layer specified."
+            )


This should be a click.UsageError, which has a much nicer UX. Compare these error messages:

RuntimeError
INFO:capellambse.cli_helpers:Loading model from [$WORKSPACE]/models/ife-demo Traceback (most recent call last): File "", line 198, in _run_module_as_main File "", line 88, in _run_code File "[$WORKSPACE]/rostools/capella_ros_tools/__main__.py", line 234, in cli() ~~~^^ File "[$WORKSPACE]/rostools/.venv/lib/python3.13/site-packages/click/core.py", line 1462, in __call__ return self.main(*args, **kwargs) ~~~~~~~~~^^^^^^^^^^^^^^^^^ File "[$WORKSPACE]/rostools/.venv/lib/python3.13/site-packages/click/core.py", line 1383, in main rv = self.invoke(ctx) File "[$WORKSPACE]/rostools/.venv/lib/python3.13/site-packages/click/core.py", line 1850, in invoke return _process_result(sub_ctx.command.invoke(sub_ctx)) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^ File "[$WORKSPACE]/rostools/.venv/lib/python3.13/site-packages/click/core.py", line 1246, in invoke return ctx.invoke(self.callback, **ctx.params) ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "[$WORKSPACE]/rostools/.venv/lib/python3.13/site-packages/click/core.py", line 814, in invoke return callback(*args, **kwargs) File "[$WORKSPACE]/rostools/capella_ros_tools/__main__.py", line 196, in export raise RuntimeError( "Neither config nor root package nor layer specified." ) RuntimeError: Neither config nor root package nor layer specified.

UsageError
INFO:capellambse.cli_helpers:Loading model from [$WORKSPACE]/models/ife-demo Usage: python -m capella_ros_tools export [OPTIONS] Try 'python -m capella_ros_tools export --help' for help.
Error: Neither config nor root package nor layer specified.

IMO we should also use the parameter names in the error message:

Suggested change

raise RuntimeError(

"Neither config nor root package nor layer specified."

)

raise RuntimeError(

"One of --config=, --root= or --layer= is required."

)

Wuestengecko · 2025-12-05T11:03:39Z

capella_ros_tools/exporter.py

+        name = re.sub("[^a-z0-9_]", "_", name)
+        name = re.sub("^[^a-z]+", "", name)
+        name = re.sub("_+", "_", name)
+        return re.sub("_$", "", name)


Suggested change

name = re.sub("[^a-z0-9_]", "_", name)

name = re.sub("^[^a-z]+", "", name)

name = re.sub("_+", "_", name)

return re.sub("_$", "", name)

name = re.sub("[^a-z0-9]+", "_", name)

name = re.sub("^[^a-z]+", "", name)

return re.sub("_+$", "", name)

Wuestengecko · 2025-12-05T11:08:01Z

capella_ros_tools/exporter.py

+        if not camel_case_name or not camel_case_name[0].isalpha():
+            camel_case_name = "A" + camel_case_name


In make_snake_case we're stripping invalid characters from the beginning, while here in make_camel_case we're prepending a capital "A" in such cases. Does this have a specific reason? If not, IMO we should have the same behavior in both cases (either both strip, or both prepend).

Wuestengecko · 2025-12-05T11:09:11Z

capella_ros_tools/exporter.py

+
+    @staticmethod
+    def make_snake_case(name: str) -> str:
+        """Convert all cases to snake_case."""


I'd like to see some basic unit tests for make_snake_case and make_camel_case. IMO they're complex enough to warrant that.

Wuestengecko · 2025-12-05T11:46:20Z

capella_ros_tools/exporter.py

+        name = re.sub("([a-z0-9])([A-Z])", r"\1_\2", name)
+        name = re.sub("(.)([A-Z][a-z]+)", r"\1_\2", name)
+        name = name.lower()
+        # Replace invalid characters with underscores
+        name = re.sub("[^a-z0-9_]", "_", name)
+        name = re.sub("^[^a-z]+", "", name)
+        name = re.sub("_+", "_", name)
+        return re.sub("_$", "", name)


This can lead to empty names:

from capella_ros_tools.exporter import RosExportHelper as e print(repr(e.make_snake_case("1+1=2"))) # '' print(repr(e.make_snake_case("A 1+1=2"))) # 'a_1_1_2'

Wuestengecko · 2025-12-05T13:12:33Z

README.md

+````yaml
+packages:
+    asdf: <capella_pkg_uuid>
+built_ins:
+    ros_pkg: <capella_build_in_pkg_uuid>
+custom_pkg:
+    "abc":
+      - <capella_cls1_uuid>
+    "xyz":
+      - <capella_cls2_uuid>
+custom_types:
+    Bitset16: int16
+    Bitset32: int32
+````


The world needs more TOML.

Advantages over YAML:

it's not YAML

tomllib is in stdlib since Python 3.11 (previously tomli on PyPI)

very simple and very explicit format

no significant whitespace, which is especially beneficial with the config templating feature

Disadvantages:

only 4 different types of strings instead of 9

can't easily introduce an ACE by accidentally using load() instead of safe_load()

````toml [packages] asdf = "<capella_pkg_uuid>" [built_ins] ros_pkg = "<capella_built_in_pkg_uuid>" [custom_pkg] abc = ["<capella_cls1_uuid>"] xyz = [ "<capella_cls2_uuid>", "<capella_cls3_uuid>", ] [custom_types] Bitset16 = "int16" Bitset32 = "int32" ````

micha91 added 11 commits September 17, 2025 10:08

fix: Don't crash if a type is not set for a property

93a8768

feat: Add envvars and a first version of the ci-template

2bba1cb

feat: Add dependency support for non-standard branches and enum fixes

cdf4d37

# Conflicts: # capella_ros_tools/importer.py

feat: Add a new exporter based on a config file

a74346f

feat: Generate package.xml and CMake files

5f89261

feat: Make cmake generation optional

8975701

fix: Reference cross package messages correctly

27f7b1b

fix: Don't rename build-in messages

07770e2

refactor: Use new exporter for all exports while keeping the old API

37ae444

fix: Minor fixes from field tests

c7cd358

chore: Minor changes for new pre commit hooks

b4b38be

Wuestengecko reviewed Sep 17, 2025

View reviewed changes

micha91 and others added 4 commits September 17, 2025 17:28

chore: Apply suggestions from code review

33ef4ca

Co-authored-by: Martin Lehmann <martin.lehmann@deutschebahn.com>

chore: Apply more suggestions from review

a927b0b

fix: Some minor fixes and restructuring

7083d53

refactor: Restructure exporter class and general clean-up

f98e3e6

micha91 force-pushed the add-configured-exporter branch from abb5919 to f98e3e6 Compare December 3, 2025 15:22

micha91 requested a review from Wuestengecko December 3, 2025 15:30

Wuestengecko reviewed Dec 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite of Exporter, Minor Changes in Importer#15

Rewrite of Exporter, Minor Changes in Importer#15
micha91 wants to merge 15 commits intomasterfrom
add-configured-exporter

micha91 commented Sep 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Wuestengecko left a comment

Uh oh!

Wuestengecko Dec 5, 2025

Uh oh!

Wuestengecko Dec 5, 2025

Uh oh!

Wuestengecko Dec 5, 2025

Uh oh!

Wuestengecko Dec 5, 2025

Uh oh!

Wuestengecko Dec 5, 2025

Uh oh!

Wuestengecko Dec 5, 2025

Uh oh!

Wuestengecko Dec 5, 2025

Uh oh!

Wuestengecko Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if not camel_case_name or not camel_case_name[0].isalpha():
		camel_case_name = "A" + camel_case_name

Conversation

micha91 commented Sep 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Wuestengecko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants