[Proposal] Allow factory methods in ConnectionTypeList
As part of the implementation, I've prototyped the following improvement. Before finalizing (cleaning up) the implementation, I'd like to confirm that the idea is acceptable - if so, then a regular PR review will be done on cleaned-up code.
I am playing with an alternative implementation of XPressNet support. I have Nano-X (home) and DR-5000; Nano-X (supported by JMRI) does not work properly, DR-5000 (not listed ! as supported for XpressNet at the moment) also has some issues.
I write necessary adjustments in new types of connection. Part of the fixes supposedly apply to Lenz LZV100; so some experimental connections for that type as well (the fixes are not easy to backport - and possibly not desirable either, for stability reasons).
NanoX and DR5k basically use the same settings for serial or network communication *as already implemented* for LI100, LIUSB or LIUSBEthernet; they also use the same message encapsulation. That makes:
Nano-X: GEN-LI (both Paco Canada's design)
DR-5000: Li-USB, Li-USB-Ethernet
LZV100 alternative: LI100, LI-USB, LI-USB-Ethernet
I don't know who would run NanoX one with Lenz LI100 or LI-USB, but it could be possible ;) So let's have 6 ConnectionConfig classes.
In the current nomenclature, following example of LI100, LI101, LI100F and others one has to create ConnectionConfig subclass to:
* create Adapter class
* interface with user for configuration
With each ConnectionConfig, a (de)serializer class must exist. In my specific case, no options are added. The only relevant behavioral change of ConnectionConfig is the adapter subclass creation.
From the configuration perspective, despite 6 ConnectionConfigs, I have just two protocol families:
* serial/USB (1st superclass)
* network TCP/IP (2nd superclass)
That's 12 classes, essentially empty (all work done by superclasses), except for:
- factory method
But to support UI configuration + adapter creation, I need "just" need two parametrized classes: each inherits the relevant UI implementations, parameter drives the appropriate Adapter creation.
Instead of requring several n*2 (empty) classes, to just encode a single constant, and a single creation, I'd like to propose to **support static factory methods** in ConnectionTypeList.getAvailableProtocolClasses.
If used, those 12 classes will reduce to 4; There would not have to be 6 ConnectionConfig classes, but just 2 of them.
The current syntax for Strings returned from getAvailableProtocolClasses is "fully qualified java class name". I propose to change it to:
item := className | className ':' methodName
className := javaIdentifier | javaIdentifier '.' javaIdentifier
methodName := javaIdentifier
So the line could read:
The change should be backwards compatible (changed implementation will read old data), but older implementation will refuse to load new connection configs that use this feature.
Prototypical implementation is here: https://github.com/svatoun/JMRI/blob/prototype/xpressnet-sequence-fix3/java/src/jmri/jmrix/JmrixConfigPane.java#L285
Serial-style adapter, that works for all serial AND usb flavours (= 5 ConnectionTypes) is at https://github.com/svatoun/JMRI/blob/prototype/xpressnet-sequence-fix3/java/src/jmri/jmrix/lenzplus/config/LenzSerialConnectionConfig.java
Persistence is done as usual, but with an additional attribute that creates the appropriate Adapter:
Of course I can make a copy-paste of the ConnectionConfig + serializer into 12 classes. But somehow I feel there's already more than enough code duplications in the codebase.
- is such an extension acceptable ?
- is the proposed syntax acceptable ?
- are there other parts of code that interpret ConnectionTypeList data I am not aware of (= may need an update) ?
Waiting for feedback,