Add initial setup for osirules checker

[pmai]

No description provided.

[TimmRuppert]

We should consider trying to extract the version from the trace file name and only fall back to OSI_FALLBACK_VERSION if that extraction fails. This approach would make it easier to use the tool from the CLI (at a later stage) without requiring the user to explicitly specify the version. Of course, providing the version explicitly via an CLI argument could still remain a valid option.

[pmai]

Yes, that would be the idea, however I wanted to delay that code to the point in time when we integrate MCAP support, as we can then do it generically, rather than just special coding for the single-trace files today.

[TimmRuppert]

While we agreed to use the .yml files internally and largely hide them from the user, I see potential in allowing their use for custom rule definitions. This could enable users to define additional .yml files to check not only for standard conformity but also for custom requirements - such as a minimum number of detections, the presence of specific fields, and so on. These files could also be exchanged between collaborating companies prior to exchanging trace files.

We could reuse a lot of the present code and might want to restructure it now already.

[pmai]

Again, I agree that this is the idea - the change should be trivial already (just the option), but I'd postpone this to a new PR, as this is a simple orthogonal change.

[pmai]

Given I was reorganizing the cli interface for better interactive use already - due to other requirements, I implemented this change now as well, as it was easier to do this way.

[TimmRuppert]

Great!

I tested the CLI with a custom trace file and the default config and naively did not specify the trace files type. This causes an exception (KeyError) as config.get_config_param("osiType") is None an thus this line fails:

trace = OSITrace(
        config.get_config_param("InputFile"), config.get_config_param("osiType")
    )

As you noted here that you'd prefer to postpone the "guessing" of the trace file type, I suggest we add a simple check for it during argument parsing in args_entrypoint() for now.

    parser.add_argument(
        "--output_config",
        type=pathlib.Path,
        help="Path to save the configuration after running the checks.",
    )
+   args = parser.parse_args()

+   if args.input_file and not args.osiType:
+       parser.error("argument --input_file requires --osiType to also be specified")

-   return  parser.parse_args()        
+   return args

[pmai]

The check actually needs to be a check on the config, as the source of osiType, like inputFile, can be either a loaded config or argument handling or both.

That said none of the checkers in the framework currently do not do any validation on the config, blindly assuming that e.g. inputFile will be set. So there is probably room to debate how the bundles should treat missing required config settings, so that these are aligned in their treatment.

[pmai]

In any case there is a fallback to SensorView in the current code, which is just not consistently used - hence will rework that to be more apparent.

[TimmRuppert]

So there is probably room to debate how the bundles should treat missing required config settings, so that these are aligned in their treatment.

Agreed. I just wanted to highlight that is easy to cause general exceptions arbitrarily. This should be avoided, as it can leave users unaware of what went wrong in terms of configuration without access to the source code.

[TimmRuppert]

The documentation states:

A check should address exactly one rule, unless there is a strong reason not to. In exceptional cases, a check may address multiple rules, or multiple checks may be required to check a particular rule.

While this technically allows multiple rules, it also spams the log:

2025-06-24 14:00:13,496 - There are 175 rules registered to the check osirules_osi. A check should address exactly one rule, unless there is a strong reason not to. See the following document for more information: https://github.com/asam-ev/qc-framework/blob/main/doc/manual/checker_library.md#check-characteristics

There is also this issue: asam-ev/qc-framework#163

Should we then not only create rules dynamically but also checks?

[pmai]

There is some discussion on the qc framework side whether this rule "1 check per 1 rule" should not be loosened more, in which case the warning would be removed.

That said we might want to have some more fine-granular checkers than just one, however it is unclear that we really want to have one per rule, as that is likely to grow the output file substantially, and makes the overview potentially unwieldy - since we have potentially better ways to e.g. remove a single rule from consideration, this might also agitate against having strictly 1:1 mappings here. Maybe group by field, for example.

Given this uncertainty I would stay with the one checker for now, as we can easily split up later on (either dynamically generating the checkers/checkerids, and/or grouping them usefully), which would require some discussion that we should have going forward.

[TimmRuppert]

This check is already included in deserialization_osi should it not occur in only one place? This also causes the issues to be reported twice

[pmai]

Yes, the idea would be to likely remove that check going forward from the deserialization checker bundle - either because we fold the whole deserialization checker bundle into the osirules bundle, or we specialize the deserialization checker bundle to focus on pure deserialization issues.

[AsamDiegoSanchez]

No further inputs. Decision from the 08.07.2025 OSI multi-trace meeting: ready to be merged

[msandrk-avl]

Do we want to allow leaving out osiType parameter from the config.xml or not? Line 364 seems to suggest that we do allow the this and in that case we default to SensorView. If so, this line should be changed to trace = OSITrace(config.get_config_param("InputFile"), expected_type_name) (use variable defined above).

Currently, leaving out the osiType parameter in the input config file would result in a KeyError: None.

[pmai]

Yep, that was a carryover mistake, now fixed.

[pmai]

BTW: Not that the SensorView default currently is the best idea - we probably want to default to auto-detection in the future, with an error fallback when auto detect does not work - but that should be handled in a separate PR that introduces multi-trace file handling.

[jdsika]

Maybe add "the warning"