sharpbrick / powered-up

.NET implementation of the LEGO PoweredUp Protocol
MIT License
94 stars 19 forks source link

Device discovery still uses static port information #195

Closed mennolodder closed 5 months ago

mennolodder commented 6 months ago

When running device list or device dump-static-port the application seems to send all the port information requests but still uses the old static info.

See the situation below where the motor had 6 modes in the static port info, but 5 in reality. Eventhough the motor reported 5 modes earlier, the 6th (index 5) is also requested and results in an error.

It goes wrong here:

dbug: SharpBrick.PoweredUp.Bluetooth.BluetoothKernel[0]
      > 06-00-22-00-05-00
dbug: SharpBrick.PoweredUp.Bluetooth.BluetoothKernel[0]
      < 05-00-05-22-06
info: SharpBrick.PoweredUp.Protocol.LegoWirelessProtocol[0]
      Received 05:00:05:22:06
fail: SharpBrick.PoweredUp.Functions.TraceMessages[0]
      Error - InvalidUse from PortModeInformationRequest

The reply translates as: Error - Port Mode Information Request - Port 0 - Invalid use (e.g. parameter error(s)

Which makes sense, because it is requesting mode 5 (so the 6th mode) where earlier the device announced 5 modes (( checked, the bold number is 'Total Mode Count'_:

dbug: SharpBrick.PoweredUp.Bluetooth.BluetoothKernel[0]
      > 05-00-21-00-01
dbug: SharpBrick.PoweredUp.Bluetooth.BluetoothKernel[0]
      < 0B-00-43-00-01-0F-**05**-1E-00-1F-00
info: SharpBrick.PoweredUp.Functions.TraceMessages[0]
      Port Information - Port 0/0 Total Modes 5 / Capabilities Output:True, Input:True, LogicalCombinable:True, LogicalSynchronizable:True / InputModes: 1E, OutputModes: 1E

I didn't doublecheck this, but this probably is because the TechnicLargeLinearMotor.GetStaticPortInfoMessages announces 6 modes: 0B-00-43-00-01-0F-06-1E-00-1F-00

For my motor it outputs this version info. could this be two versions?

     < 0F-00-04-00-01-2E-00-00-10-00-00-00-10-00-00
info: SharpBrick.PoweredUp.Functions.TraceMessages[0]
      Attached IO - Port 0/0 of device type TechnicLargeLinearMotor (HW: 0.0.0.1000 / SW: 0.0.0.1000)

If I remove the static port info messages it works and I get:

0B-00-43-00-01-0F-05-1E-00-1F-00
07-00-43-00-02-0E-00
11-00-44-00-00-00-50-4F-57-45-52-00-00-00-00-00-00
0E-00-44-00-00-01-00-00-C8-C2-00-00-C8-42
0E-00-44-00-00-02-00-00-C8-C2-00-00-C8-42
0E-00-44-00-00-03-00-00-C8-C2-00-00-C8-42
0A-00-44-00-00-04-50-43-54-00
08-00-44-00-00-05-00-10
0A-00-44-00-00-80-01-00-01-00
11-00-44-00-01-00-53-50-45-45-44-00-00-00-00-00-00
0E-00-44-00-01-01-00-00-C8-C2-00-00-C8-42
0E-00-44-00-01-02-00-00-C8-C2-00-00-C8-42
0E-00-44-00-01-03-00-00-C8-C2-00-00-C8-42
0A-00-44-00-01-04-50-43-54-00
08-00-44-00-01-05-10-10
0A-00-44-00-01-80-01-00-04-00
11-00-44-00-02-00-50-4F-53-00-00-00-00-00-00-00-00
0E-00-44-00-02-01-00-00-B4-C3-00-00-B4-43
0E-00-44-00-02-02-00-00-C8-C2-00-00-C8-42
0E-00-44-00-02-03-00-00-B4-C3-00-00-B4-43
0A-00-44-00-02-04-44-45-47-00
08-00-44-00-02-05-08-08
0A-00-44-00-02-80-01-02-04-00
11-00-44-00-03-00-41-50-4F-53-00-00-00-00-00-00-00
0E-00-44-00-03-01-00-00-34-C3-00-00-33-43
0E-00-44-00-03-02-00-00-48-C3-00-00-48-43
0E-00-44-00-03-03-00-00-34-C3-00-00-33-43
0A-00-44-00-03-04-44-45-47-00
08-00-44-00-03-05-08-08
0A-00-44-00-03-80-01-01-03-00
11-00-44-00-04-00-4C-4F-41-44-00-00-00-00-00-00-00
0E-00-44-00-04-01-00-00-00-00-00-00-FE-42
0E-00-44-00-04-02-00-00-00-00-00-00-C8-42
0E-00-44-00-04-03-00-00-00-00-00-00-FE-42
0A-00-44-00-04-04-50-43-54-00
08-00-44-00-04-05-08-08
0A-00-44-00-04-80-01-00-01-00

Here are the differences with what is checked in: image

image

mennolodder commented 6 months ago

Quoted from https://github.com/sharpbrick/powered-up/issues/58#issuecomment-1891022068 :

This should not be. Either we discover the device OR use static port inform messages. Do not remember why the behavior is like that. Device enumeration not necessarily need to discover the devices but my just pretty print our existing knowledge.

When doing device dump-static-port -p 0. I see that the first message we receive is:

< 0F-00-04-00-01-2E-00-00-10-00-00-00-10-00-00

Which is HubAttachedIOForAttachedDeviceMessage. This message is handled in the KnowledgeManager in ApplyDynamicProtocolKnowledge (called from LegoWirelessProtocol.ConnectAsync(). This is implemented as follows:

            case HubAttachedIOForAttachedDeviceMessage msg:
                hub = knowledge.Hub(msg.HubId);
                port = knowledge.Port(msg.HubId, msg.PortId);

                ResetProtocolKnowledgeForPort(msg.HubId, port.PortId, knowledge);
                port.IsDeviceConnected = true;
                port.IOTypeId = msg.IOTypeId;
                port.HardwareRevision = msg.HardwareRevision;
                port.SoftwareRevision = msg.SoftwareRevision;

                AddCachePortAndPortModeInformation(msg.IOTypeId, msg.HardwareRevision, msg.SoftwareRevision, hub, port, knowledge, deviceFactory);

Which loads the static information.

I don't know much about this part yet, but for port / device discovery it would be clean to not load the static information. However we could also make this more robust by making sure the info is overwritten when never info is received (that might lead to a less consistent state though).

The first option seems best, we could make that a protocol feature (use dumps or discover)

tthiery commented 6 months ago

Discovery should start fresh. And an explicit operation. The protocol is IMHO from LEGO itself used without discovering the knowledge. Discovery takes far too long for a regular protocol usage, no matter use the case. 30 seconds or what it is, is just too long to start a remote. Discovery is for development or unknown devices.

tthiery commented 5 months ago

Thanks for fixing this.