Export the modulename to the extras-config yaml

[BLuedtke]

As referenced in an issue in SWTbahn-CLI, where we use BahnDSL: It would be very helpful if BahnDSL would export the module name (given in e.g. this line where the module starts) to the yaml config files somewhere.
The modulename would serve as an identifier of what platform/model railway is being configured/described in the .bahn file, and would thus be useful in the swtbahn-cli server downstream to know what platform the server is running for/on.

I thought that the "extras-config" yaml file would be a good place to export the module name to. However, we could also consider exporting it to every generated yaml file, such that one can check whether they all belong to the same config. But I don't really see that as an important use case, and it would generate more work in implementing the parsers.
I'll add another comment below with my prototype implementation of how the respective yaml exporter in BahnDSL could be adjusted to export the module name (once that is ready).

[BLuedtke]

Alright, this was my basic idea of how the module name could be exported to the extras-config yaml:

If no module name is specified, this defaults to saying "modulename: undefined" in the extras-config. Any opinions on that?

Also: All the other sections are exported via a separate "exportSection", i.e. with its own method in a separate file. My prototype here just puts the module name export directly in the main extras-config yaml exporter. Is that okay or should this also be put into a separate file? I figured that its such a small change, might as well do it directly.

[eyip002]

Seems like a sensible location.
I don't think the null check is necessary, because the xtext grammar requires all root modules to have a name (otherwise there is a syntax error and compilation won't be triggered).

bahndsl/src/de.uniba.swt.dsl/src/de/uniba/swt/dsl/Bahn.xtext

Lines 15 to 18 in b24a3ab

	RootModule:
	'module' name=ID
	(properties+=ModuleProperty)*
	'end';

[BLuedtke]

I initially had no null check, but then a lot of the tests failed.
I think this happens because in some .bahn files, there's only function definitions/implementations for e.g. the interlocking, but no configuration. Thus there's no "module" element in the file (cf. this test file), but there appears to still be a "root module", and when the name property is accessed that results in an exception.

[eyip002]

Hmm, but that function definition should be matched by the following rules:
BahnModel -> Component -> FuncDecl (not RootModule)

[BLuedtke]

Yeah I was also surprised, but I haven't fully understood all the rules/paths/etc. that would have to be considered here, but I did notice that all tests were happy once I added that null check. I can re-run them in the future to look at them again, currently not at the computer where I have BahnDSL dev environment set up.

[eyip002]

On the naming of the yaml keys: the convention in the other yaml files has been to separate words using "-", but some inconsistency has crept into the extras file:

• signaltypes -> signal-types
• peripheraltypes -> peripheral-types
• modulename -> module-name

bahndsl/src/de.uniba.swt.dsl/src/de/uniba/swt/dsl/common/generator/yaml/ExtrasYamlExporter.java

Lines 97 to 99 in 0a0cf35

	exportSection("signaltypes:", signaltypes);
	exportSection("compositions:", compositeSignals);
	exportSection("peripheraltypes:", peripheraltypes);

[BLuedtke]

Interesting. I agree, even outside of the conventions-standpoint, module-name feels better than just modulename.

[eyip002]

Yeah I was also surprised, but I haven't fully understood all the rules/paths/etc. that would have to be considered here, but I did notice that all tests were happy once I added that null check. I can re-run them in the future to look at them again, currently not at the computer where I have BahnDSL dev environment set up.

I would be interested in seeing an example where we have a module without a proper name 😂

[BLuedtke]

I removed the null-check and built the command line compiler (sh build-gradle.sh). I then ran the unit tests (sh build-test.sh) and integration tests (sh build-test-cli.sh) as specified in this file. (there's an sh missing for the command to run the cli integration tests in that Maintenance.md file, by the way).

All those tests pass.

Which surprised me, as I thought that surely I had experienced failing tests previously when developing the prototype. Maybe I hadn't built it correctly? Something cached? Maybe I'm not building it correctly right now? - I'll try out some more.

[BLuedtke]

I now understand what happened. I added a log statement in a different place in BahnDSL, where I was logging the name of the rootModule, that I was accessing via some workaround. THERE I had to add the NULL-check to prevent a crash, not in the extras-config exporter. Probably because in the location where I added the log, the root module was not actually being used, as it was code that dealt with both .bahn files with modules (config) and .bahn files with interlocking/drive route implementation, i.e., with no modules.
So we could remove the null-check then. I ran the unit and integration tests with the null check removed, and all tests passed. One could argue to leave the check in place in the spirit of extra-defensive programming. Thoughts?

[eyip002]

The extra null check would just be dead code: the parser is responsible for ensuring that all modules have a name. When exporting an extras-config yaml file, you can safely assume that rootModule.getName() is never null.

[BLuedtke]

Good, then no null check. Next question would be, should a new release be made with just this change? I don't know how much effort that is. But an increase in just the last semver number should be fine, I thought. I thought about also updating the Kieler dependencies (BahnDSL is using version 1.3 I believe) to the current 1.5. But no idea if there's breaking changes and I don't really have the time to test all that.

[trinnguyen]

Hi @BLuedtke , having a new release should be fine, just increasing the sem ver of both bahnc cli and the Bahn IDE (Eclipse-based).

Xtext and Kieler dependencies upgrade can be ignored for now, as long as we can build the project. They can be addressed later.

[eyip002]

@BLuedtke are you also planning to update the naming of yaml keys signaltypes and peripheraltypes? You'll need to update all the extras files in SWTbahn-cli and its associated parser anyway.

[BLuedtke]

@BLuedtke are you also planning to update the naming of yaml keys signaltypes and peripheraltypes? You'll need to update all the extras files in SWTbahn-cli and its associated parser anyway.

true, might as well.
Edit: Alright, renamed; tests are still passing. And testing manually with some configs also gives the expected results. I'll open a pull request then.