JMRI / JMRI

JMRI model railroad digital command & control software
https://www.jmri.org
Other
235 stars 330 forks source link

Registration of generic consist managers needs to be updated to current standards. #4203

Closed pabender closed 6 years ago

pabender commented 6 years ago

When the consist manager system was put into place, two generic consist managers (which now are jmri.managers.DccConsistManager and jmri.managers.NmraConsistManager.)

DccConsistManager is associated with the default AddressedProgrammer. and this was originally enforced through the call to jmri.InstanceManager.setAddressedProgrammer() (see https://github.com/JMRI/JMRI/blob/134a98d7243a86298c7487256c2525b6a77342dc/java/src/jmri/InstanceManager.java#L723)

NmraConsistManager is assoicated with the default CommandStation and this was originally enforced through the call to jmri.InstanceManager.setCommandStation() (see https://github.com/JMRI/JMRI/blob/134a98d7243a86298c7487256c2525b6a77342dc/java/src/jmri/InstanceManager.java#L668 ).

Now that we are using jmri.InstanceManager.setDefault and jmri.InstanceManager.store() to store lists of classes, we no longer automatically install either of these connection independent consist managers (or at least I can't see where we install either of them).

So what is the best approach in the current scheme of storing instances in the instance manger to make either of these two defaults available? It would also be nice if we could choose one of these two as the default (over any system specific ConsistManager) on systems that provide the necessary infrastructure, but that is a new feature.

Paul

rhwood commented 6 years ago

Neither of those two calls are deprecated, and should continue to be used. The following grep outputs show where they are used:

$ grep -nr InstanceManager.setAddressedProgrammerManager java/src/
java/src//jmri/jmrix/qsi/QsiSystemConnectionMemo.java:100:        InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/can/cbus/CbusConfigurationManager.java:54:            InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/roco/z21/Z21XNetInitializationManager.java:43:            jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/ztc/ztc611/ZTC611XNetInitializationManager.java:40:            jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/sprog/SprogSystemConnectionMemo.java:238:            jmri.InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/nce/NceSystemConnectionMemo.java:218:                InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/mrc/MrcSystemConnectionMemo.java:164:            InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/zimo/Mx1SystemConnectionMemo.java:144:            InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/openlcb/OlcbConfigurationManager.java:71:            InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/openlcb/OlcbSystemConnectionMemo.java:88:            InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/lenz/XNetInitializationManager.java:39:                jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/lenz/XNetInitializationManager.java:88:                    jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/lenz/XNetInitializationManager.java:122:                    jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/lenz/XNetInitializationManager.java:137:                    jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/lenz/li100/LI100XNetInitializationManager.java:38:                jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/lenz/li100/LI100XNetInitializationManager.java:85:                    jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/lenz/li100/LI100XNetInitializationManager.java:114:                    jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/lenz/li100/LI100XNetInitializationManager.java:127:                    jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/lenz/hornbyelite/EliteXNetInitializationManager.java:44:            jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/tams/TamsSystemConnectionMemo.java:64:        InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/easydcc/EasyDccSystemConnectionMemo.java:69:        InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/dccpp/DCCppInitializationManager.java:58:            jmri.InstanceManager.setAddressedProgrammerManager(systemMemo.getProgrammerManager());
java/src//jmri/jmrix/loconet/hexfile/HexFileServer.java:54:            jmri.InstanceManager.setAddressedProgrammerManager(port.getSystemConnectionMemo().getProgrammerManager());
java/src//jmri/jmrix/loconet/hexfile/HexFileFrame.java:179:            jmri.InstanceManager.setAddressedProgrammerManager(port.getSystemConnectionMemo().getProgrammerManager());
java/src//jmri/jmrix/loconet/pr2/PR2SystemConnectionMemo.java:41:            InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/loconet/pr3/PR3SystemConnectionMemo.java:76:            InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/loconet/pr3/PR3SystemConnectionMemo.java:195:            InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
java/src//jmri/jmrix/loconet/LocoNetSystemConnectionMemo.java:249:            InstanceManager.setAddressedProgrammerManager(getProgrammerManager());
$ grep -nr InstanceManager.setCommandStation java/src/
java/src//jmri/jmrix/can/cbus/CbusConfigurationManager.java:60:        jmri.InstanceManager.setCommandStation(getCommandStation());
java/src//jmri/jmrix/roco/z21/Z21XNetInitializationManager.java:51:        InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/ztc/ztc611/ZTC611XNetInitializationManager.java:48:        jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/sprog/SprogSystemConnectionMemo.java:140:        jmri.InstanceManager.setCommandStation(commandStation);
java/src//jmri/jmrix/direct/serial/SerialDriverAdapter.java:137:        jmri.InstanceManager.setCommandStation(TrafficController.instance());
java/src//jmri/jmrix/nce/NceSystemConnectionMemo.java:94:        jmri.InstanceManager.setCommandStation(nceTrafficController);
java/src//jmri/jmrix/nce/NceTrafficController.java:513:            jmri.InstanceManager.setCommandStation(self);
java/src//jmri/jmrix/mrc/MrcSystemConnectionMemo.java:71:     jmri.InstanceManager.setCommandStation(mrcTrafficController);
java/src//jmri/jmrix/marklin/MarklinTrafficController.java:36:        jmri.InstanceManager.setCommandStation(this);
java/src//jmri/jmrix/ecos/EcosTrafficController.java:34:        jmri.InstanceManager.setCommandStation(this);
java/src//jmri/jmrix/lenz/XNetInitializationManager.java:47:            jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/lenz/XNetInitializationManager.java:96:                jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/lenz/XNetInitializationManager.java:128:                jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/lenz/XNetInitializationManager.java:145:                jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/lenz/li100/LI100XNetInitializationManager.java:46:             jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/lenz/li100/LI100XNetInitializationManager.java:93:                jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/lenz/li100/LI100XNetInitializationManager.java:120:                jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/lenz/li100/LI100XNetInitializationManager.java:135:                jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/lenz/hornbyelite/EliteXNetInitializationManager.java:38:        jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/tams/TamsTrafficController.java:38:        jmri.InstanceManager.setCommandStation(this);
java/src//jmri/jmrix/easydcc/EasyDccSystemConnectionMemo.java:81:        InstanceManager.setCommandStation(commandStation);
java/src//jmri/jmrix/dccpp/DCCppInitializationManager.java:64:        jmri.InstanceManager.setCommandStation(systemMemo.getCommandStation());
java/src//jmri/jmrix/loconet/LocoNetSystemConnectionMemo.java:129:            jmri.InstanceManager.setCommandStation(sm);

Note also that this question was asked in 20d86f3 with the thought that maybe the InstanceManager should not be trying to be smart about setting up consist managers on behalf of others.

pabender commented 6 years ago

Ok, after looking at this a little more, I think the issue may be related to #1014. But the question still remains as to what is the best way to move forward here.

I'm thinking:

  1. Set these two Consist Managers so they can be automatically created by the instance manager
  2. Allow these consist managers to be explicitly selected as the default consist manager in the defaults tab.

After we make those changes, we can deprecate the existing code.

Do those steps make sense?

bobjacobsen commented 6 years ago
  1. Set these two Consist Managers so they can be automatically created by the instance manager
  2. Allow these consist managers to be explicitly selected as the default consist manager in the defaults tab.

I don't know enough about the context of the two different managers, unfortunately. Do they require that certain objects already exist (e.g. CommandStation or an AddressedProgrammerManager? If so, the right solution (if possible) is to break that dependence so they connect to the CommandStation or AddressedProgramManager when first used, in which case we can have InstanceManager automatically create them.

Nor do I know enough about what it means to use one or the other, hence to default to one or the other. Why would a user want to care about using one or the other?

The current default structure (for e.g. CommandStation, AddressedProgrammer, etc) is by-connection: Each system connection can provide, or not provide, one item. The default is then selected from the item(s) provided by all the existing systems. But that's different here, where you're selecting from two different items, neither of which is associated with a particular system.

If there are reasons to use one on one system, and another with another, then we could have them be installed explicitly at system startup, by each system. The default system would then swing into action, though I'm not at all sure that there's a reasonable case for "we're using the ConsistManager from A, but the CommandStation from B and the AddressedProgrammer from C".

pabender commented 6 years ago

On Wed, Oct 4, 2017 at 12:12 PM, Bob Jacobsen notifications@github.com wrote:

  1. Set these two Consist Managers so they can be automatically created by the instance manager
  2. Allow these consist managers to be explicitly selected as the default consist manager in the defaults tab.

I don't know enough about the context of the two different managers, unfortunately. Do they require that certain objects already exist (e.g. CommandStation or an AddressedProgrammerManager? If so, the right solution (if possible) is to break that dependence so they connect to the CommandStation or AddressedProgramManager when first used, in which case we can have InstanceManager automatically create them.

The current implementation doesn't actually do anything with the CommandStation (for the NmraConsistManager) or AddressedProgrammerManager (for the DccConsistManager) until we actually do something with the consist. At that point, they request the default of the appropriate manager from the InstanceManager.

Nor do I know enough about what it means to use one or the other, hence to default to one or the other. Why would a user want to care about using one or the other?

The NmraConsistManager uses the NMRA Consist Creation Packet to add a locomotive to a consist. The DccConsistManager uses operations mode programming to set CV19

The NmraConsistManager is actually prefered, because there are some decoders that don't allow writing to any address CV using POM, but there are systems that support operations mode programming, but don't let you send arbitrary commands to the rails.

If systems have a way to trigger sending the Consist Creation packet without sending it explicitly, those systems SHOULD have a system specific consist manager (this is important for systems like the Lenz Compact/Atlas Commander, where you can create consists, but you can't do either ops mode programming or send arbitrary packets...)

The current default structure (for e.g. CommandStation, AddressedProgrammer, etc) is by-connection: Each system connection can provide, or not provide, one item. The default is then selected from the item(s) provided by all the existing systems. But that's different here, where you're selecting from two different items, neither of which is associated with a particular system.

If there are reasons to use one on one system, and another with another, then we could have them be installed explicitly at system startup, by each system. The default system would then swing into action, though I'm not at all sure that there's a reasonable case for "we're using the ConsistManager from A, but the CommandStation from B and the AddressedProgrammer from C".

The real issue I would like to address here is that there has been a long standing desire by users to be able to use either of the generic managers instead of the system specific consist manager. That isn't possible if we don't list the two consist managers explicitly.

This was specifically requested for LocoNet systems used on large n-trak layouts because of the limitations of the DCS100 (perhaps it's not such an issue now with the DCS240, but i don't know...). The benefit in this situation is it's possible to create the consist without taking up a slot for each locomotive. (I've run on layouts where the 120 slot limit on the DCS100 was an issue....)

Paul

bobjacobsen commented 6 years ago

The NmraConsistManager uses the NMRA Consist Creation Packet to add a locomotive to a consist. The DccConsistManager uses operations mode programming to set CV19

The NmraConsistManager is actually prefered, because there are some decoders that don't allow writing to any address CV using POM, but there are systems that support operations mode programming, but don't let you send arbitrary commands to the rails.

If systems have a way to trigger sending the Consist Creation packet without sending it explicitly, those systems SHOULD have a system specific consist manager (this is important for systems like the Lenz Compact/Atlas Commander, where you can create consists, but you can't do either ops mode programming or send arbitrary packets…)

OK, if I understand that properly, each system (hardware) implementation can decide whether the right one to use them that hardware is DCM or NCM, based on what that hardware can use to send to the rails. So we should have systems create the appropriate object as they create all their other system-specific objects. (That’s different from now)

The real issue I would like to address here is that there has been a long standing desire by users to be able to use either of the generic managers instead of the system specific consist manager. That isn't possible if we don't list the two consist managers explicitly.

I don’t think this is about the existing Defaults, in the sense of which system provides what. “Which system provides the consist manager” is a different question.

There are two different user patterns that might exist, which would lead to different solutions:

Then the default selection is just deciding on which system provides the multiple managers (including the case of just one, if there’s no system-specific one)

pabender commented 6 years ago

Sent from my iPad

On Oct 4, 2017, at 3:23 PM, Bob Jacobsen notifications@github.com wrote:

The NmraConsistManager uses the NMRA Consist Creation Packet to add a locomotive to a consist. The DccConsistManager uses operations mode programming to set CV19

The NmraConsistManager is actually prefered, because there are some decoders that don't allow writing to any address CV using POM, but there are systems that support operations mode programming, but don't let you send arbitrary commands to the rails.

If systems have a way to trigger sending the Consist Creation packet without sending it explicitly, those systems SHOULD have a system specific consist manager (this is important for systems like the Lenz Compact/Atlas Commander, where you can create consists, but you can't do either ops mode programming or send arbitrary packets…)

OK, if I understand that properly, each system (hardware) implementation can decide whether the right one to use them that hardware is DCM or NCM, based on what that hardware can use to send to the rails. So we should have systems create the appropriate object as they create all their other system-specific objects. (That’s different from now)

The system as it is implemented now was designed so that new system implementations didn't have to think about it unless they needed to install a system specific consist manager. This is the reason why consists created by the generic consist managers use the default CS or APM instead of being registered against a specific CS or APM. ( it is actually the consist objects that do the communication with the layout. The managers just create those consists. ).

I still think the current design is the right design choice, and it's certainly an example of DRY system design.

Paul

bobjacobsen commented 6 years ago

The system as it is implemented now was designed so that new system implementations didn't have to think about it unless they needed to install a system specific consist manager. This is the reason why consists created by the generic consist managers use the default CS or APM instead of being registered against a specific CS or APM. ( it is actually the consist objects that do the communication with the layout. The managers just create those consists. ).

So where does the knowledge live about whether the NCM and/or GCM will work?

Perhaps I misunderstand, but it sounds like the user has to figure that out. That’s a DRY failure by a factor of hundreds, if not thousands.

pabender commented 6 years ago

On 10/04/2017 08:58 PM, Bob Jacobsen wrote:

The system as it is implemented now was designed so that new system implementations didn't have to think about it unless they needed to install a system specific consist manager. This is the reason why consists created by the generic consist managers use the default CS or APM instead of being registered against a specific CS or APM. ( it is actually the consist objects that do the communication with the layout. The managers just create those consists. ).

So where does the knowledge live about whether the NCM and/or GCM will work?

When you register an Addressed Programmer, you ALSO register the DccConsistManager.

When you register a Command Station, you ALSO register the NMRAConsistManager.

So, it's the simple fact of having an Addressed Programmer or a Command Station that tells you if it will work. The code itself is only dependent on defaults for these objects existing. The code that depends on the default isn't triggered until you try to add something to a consist.

Perhaps I misunderstand, but it sounds like the user has to figure that out. That’s a DRY failure by a factor of hundreds, if not thousands.

I think you do misunderstand. The user DOESN'T have to figure anything out in the current system, AND the programmer doesn't have to do anything other than register the addressed programmer or command station object.

Now, perhaps we could move the registration to some common point outside the instance manager, but moving that code into the locations where individual systems register the CS or APM is a non-starter.

As it stands now, I could make either one of these into an InstaceManagerAutoDefault pretty easily.

Paul

bobjacobsen commented 6 years ago

The problem seems to shift from one post to another, so I'll just step back now.

pabender commented 6 years ago

On 10/04/2017 10:54 PM, Bob Jacobsen wrote:

The problem seems to shift from one post to another, so I'll just step back now.

Sorry, I don't mean to make it sounds like things are shifting. I'm just leery of taking an object that has been system independent and making it dependent on a system specific object. That certainly is an alternative, and I'm contemplating how to do that without repeating the same code all over the system.

How about this idea:

1) We set some variation of the existing consist managers up as an InstanceManagerAutoDefault to preserve the "programmers don't have to remember to do this" aspect of the current system. I'm thinking this would be a new consist manager that will check the defaults and create an NMRA consist if a default Command Station Exists and it will create a DccConsist if a command station doesn't exist, but a default Addressed Programmer does. if neither exists, it can return an error back to the Consisting tool (there are provisions for this already).

2) Add constructors to the two existing consist managers that take an appropriate object as a parameter. These version can then be registered by a system as appropriate so the defaults can be selected. This will require a little restructuring of the consists as well, but it won't be significant.

Does that sound like a reasonable approach to migrating the default consist managers to the current thinking behind the InstanceManager?

Then we can deal with the second issue later (i.e. if a system registers more than one, how do we let the user decide which to use).

Paul

pabender commented 6 years ago

What drove this to being labeled as a bug turns out to be an incomplete system implementation.

There is still a change required to bring the consist managers in line with current practice.

dheap commented 6 years ago

I'm a bit lost following all this but I gather we are mainly talking about the two generic consist managers?

NCE still needs its own Consist Manager that triggers consist operations via an NCE interface command, which both updates command station internal tables and generates NMRA Consist Control packets. I gather that the XPressNet and LocoNet Consist Managers have similar requirements and so require their own Consist Managers

Do I have this right?

dheap commented 6 years ago

On 5 Oct 2017, at 3:03 am, Paul Bender paul.bender@acm.org [jmriusers] jmriusers@yahoogroups.com wrote:

For systems that provide a Command Station interface (i.e. they let us send raw DCC packets to the rails) we have the NMRAConsistManager. This version sends the Consist Control Packet.

Here again NCE systems are the exception to the rule. While NceTrafficController implements CommandStation, only the Power Pro (serial) connection has a command for sending raw DCC packets. The USB-connected systems lack such a command and we simply log an error as we have no way to return a failure via the Command Station interface.

Of course we could intercept and reverse engineer the Consist Control raw packets and instead send them via the provided NCE serial command (we already have do this for accSignalDecoderPkt, accDecoderPktOpsMode & accDecoderPktOpsModeLegacy). It's ugly but would go some way towards being able to use or extend the generic NMRAConsistManager for NCE systems.

dheap commented 6 years ago

@dheap

Of course we could intercept and reverse engineer the Consist Control raw packets and instead send them via the provided NCE serial command (we already have do this for accSignalDecoderPkt, accDecoderPktOpsMode & accDecoderPktOpsModeLegacy). It's ugly but would go some way towards being able to use or extend the generic NMRAConsistManager for NCE systems.

Not that I'd advocate such an approach. Overriding the methods in DccConsist is really the only practical way to go, particularly as we have to maintain consist position as well...

Forget that idea...

rhwood commented 6 years ago

The proper OOP and DRY way to do this is probably to move the registration of ConsistManagers into the get(T) and provides(Class) methods of jmri.jmrix.SystemConnectionMemo and ensure that all subclasses of SystemConnectionMemo are calling return super.get(T) or return super.provides(Class) as the last line of their get(T) and provides(Class) methods respectively instead of return null and return false.

This will also ensure the ManagerDefaultsSelector promotes the correct ConsistManager, since it does so by calling SystemConnectionMemo.get(ConsistManager.class) and remove a need for special handling in the InstanceManager.

Incidentally, the DCCppSystemConnectionMemo.provides(Class) method specifically states that DCC++ systems do not provide a ConsistManager, so its probably the case that JMRI should not be creating a default consist manager in this case.

pabender commented 6 years ago

Sent from my iPad

On Oct 5, 2017, at 5:42 AM, Randall Wood notifications@github.com wrote: The proper OOP and DRY way to do this is probably to move the registration of ConsistManagers into the get(T) and provides(Class) methods of jmri.jmrix.SystemConnectionMemo and ensure that all subclasses of SystemConnectionMemo are calling return super.get(T) or return super.provides(Class) as the last line of their get(T) and provides(Class) methods respectively instead of return null and return false.

This will also ensure the ManagerDefaultsSelector promotes the correct ConsistManager, since it does so by calling SystemConnectionMemo.get(ConsistManager.class) and remove a need for special handling in the InstanceManager.

Ok, let me work on that then. Incidentally, the DCCppSystemConnectionMemo.provides(Class) method specifically states that DCC++ systems do not provide a ConsistManager, so its probably the case that JMRI should not be creating a default consist manager in this case.

Actually this is incorrect.

DCC++ systems supports operations mode programming AND they support writing writing raw packets. This is exactly the type of situation the two generic consist managers were made for.

The reason the DCCppSystemConnectionMemo even mentions the Consistzmanager is that it ( along with most of the code in the package ) was derived from the XpressNet code, and XpressNet creates a system specific consist manager.

So the provides method in DCCppSystemConnectionMemo needs to have that line removed.

Paul

pabender commented 6 years ago

On Oct 5, 2017, at 2:58 AM, Dave Heap notifications@github.com wrote: I'm a bit lost following all this but I gather we are mainly talking about the two generic consist managers?

This is only about the generic consist managers.

NCE still needs its own Consist Manager that triggers consist operations via an NCE interface command, which both updates command station internal tables and generates NMRA Consist Control packets. I gather that the XPressNet and LocoNet Consist Managers have similar requirements and so require their own Consist Managers

XpressNet, Loconet, and EasyDCC have their own consist managers that are explicitly registered during system initialization.

Paul

pabender commented 6 years ago

Sent from my iPad

On Oct 5, 2017, at 4:28 AM, Dave Heap notifications@github.com wrote: On 5 Oct 2017, at 3:03 am, Paul Bender paul.bender@acm.org [jmriusers] jmriusers@yahoogroups.com wrote:

For systems that provide a Command Station interface (i.e. they let us send raw DCC packets to the rails) we have the NMRAConsistManager. This version sends the Consist Control Packet.

Here again NCE systems are the exception to the rule. While NceTrafficController implements CommandStation, only the Power Pro (serial) connection has a command for sending raw DCC packets. The USB-connected systems lack such a command and we simply log an error as we have no way to return a failure via the Command Station interface.

That isn't an exception unique to NCE.

In this case the solution is just to never register the command station interface ( or the NMRA consist manager ) on the USB connection.

Of course we could intercept and reverse engineer the Consist Control raw packets and instead send them via the provided NCE serial command (we already have do this for accSignalDecoderPkt, accDecoderPktOpsMode & accDecoderPktOpsModeLegacy). It's ugly but would go some way towards being able to use or extend the generic NMRAConsistManager for NCE systems.

If NCE provides a method to send raw packets, that is what the Command station interface does.

Paul

pabender commented 6 years ago

Closed via 4410