Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Device discovery still uses static port information #195

Closed
mennolodder opened this issue Jan 14, 2024 · 3 comments · Fixed by #201
Closed

Device discovery still uses static port information #195

mennolodder opened this issue Jan 14, 2024 · 3 comments · Fixed by #201
Assignees
Labels
bug Something isn't working investigation Stuff to investigate

Comments

@mennolodder
Copy link
Contributor

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 mennolodder changed the title Device discovery still uses Device discovery still uses static port information Jan 15, 2024
@tthiery tthiery added bug Something isn't working investigation Stuff to investigate labels Jan 15, 2024
@mennolodder
Copy link
Contributor Author

Quoted from #58 (comment) :

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
Copy link
Member

tthiery commented Jan 17, 2024

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 pushed a commit that referenced this issue Feb 11, 2024
* Added special discovery mode to protocol to prevent use of static data when discovery devices and modes.

#195 non-breaking
---------

Co-authored-by: Menno Lodder <menno@lodder>
@tthiery tthiery added this to the v5.0 (breaking) milestone Feb 11, 2024
@tthiery
Copy link
Member

tthiery commented Feb 11, 2024

Thanks for fixing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working investigation Stuff to investigate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants