auxiliary/pegasus_upb strange behaviour with USB_PORT_CONTROL switch vector

[gnthibault]

Describe the bug
I am currently developing a small software to drive a telescope setup, and the UPBV2 is a central piece of my setup.
Unfortunately, I recently came across a weird behaviour with regard to the switchvector called "USB_PORT_CONTROL"

My current strategy for every switchvector, as a client, is to send the vector to the indiserver, and then wait for the corresponding light to be set to Ok (or eventually idle, but that's another story). This is somehow an acknowledgment mechanism, and the light is set to Ok, independantly of wether any or none of the switch value have actually been changed.

Anyway, this acknowledgment strategy is working pretty ok on most of the drivers I have been playing with recently, also, it is working on the "POWER_CONTROL" and "POWER_ON_BOOT" switchvector. However, when it comes to "USB_PORT_CONTROL", I noticed that the driver is never acknowledging the switch reception in the case that the sent switchvector has no difference with the current switch values.

Second issue:
In addition, I noticed that the switchvector named "USB_HUB_CONTROL" always exhibit an "Alert" value when used. I suppose there is an issue somewhere with this feature inside the indi driver, but I am not competent enough to debug this.

Is there anyway for someone to crosscheck if the behaviour can be reproduced, and if it is expected ?

Thank you in advance for your help

To Reproduce
Exact steps to reproduce the behavior.

1. Run indi_pegasus_upb driver
2. Set USB_PORT_CONTROL switch vector, with some values for PORT_1, PORT_2, PORT3, PORT_4
3. Try to reproduce the step 2, i.e with the exact same switch values
4. You should notice that no acknowledgement is never sent for the second call

Expected behavior
I expect the driver to send a "USB_PORT_CONTROL" switchvector with a light set to "Ok" independantly of wether the latest client command did change or didn't

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

• OS: Linux rockpro64 5.15.7-1-MANJARO-ARM Make core compile #1 SMP PREEMPT Wed Dec 8 13:17:00 UTC 2021 aarch64 GNU/Linux
• Version Hash ce74163

Log Files
Edit removed useless logs

[knro]

Thank you for the report. You can look at the implementation of USBControlV2SP property and it always sends a response from what I can see either Ok or Alert depending on the result.

[gnthibault]

Thank you for the report. You can look at the implementation of USBControlV2SP property and it always sends a response from what I can see either Ok or Alert depending on the result.

Ok thank you so much for the analysis ! I'll soon write a driver with native C++ framework so that I'll be able to debug/check this fo rmyself in the future.

Sorry for the inconvenience, I'll re-re-recheck my client code / debug logs then to understand what is happening. Thank you again for your valuable help

[gnthibault]

Ok I tried to checkout the logs on driver side to see what was happening there, here is what I see in the erroneous case (ie, trying to set switch vector to the same value that the driver already has):

2021-12-26T11:47:01: Client 0: read newSwitchVector Pegasus UPB USB_PORT_CONTROL
PORT_1='On'
PORT_2='Off'
PORT_3='Off'
PORT_4='Off'
PORT_5='On'
PORT_6='Off'
2021-12-26T11:47:01: Driver indi_pegasus_upb: queuing responsible for
2021-12-26T11:47:01: Driver indi_pegasus_upb: sending msg copy 1 nq 1:


On


Off


Off


Off


On


Off

No in the "Normal" case, ie, when part of the switch vector is different, and I do get an answer from the server:

2021-12-26T11:48:42: Client 0: read newSwitchVector Pegasus UPB USB_PORT_CONTROL
PORT_1='Off'
PORT_2='Off'
PORT_3='Off'
PORT_4='Off'
PORT_5='On'
PORT_6='Off'
2021-12-26T11:48:42: Driver indi_pegasus_upb: queuing responsible for
2021-12-26T11:48:42: Driver indi_pegasus_upb: sending msg copy 1 nq 1:


Off


Off


Off


Off


On


Off

2021-12-26T11:48:42: Driver indi_pegasus_upb: read setSwitchVector Pegasus UPB USB_HUB_CONTROL Idle

++++++++++++++++++++++++++++++++++

2021-12-26T11:48:43: Driver indi_pegasus_upb: read setSwitchVector Pegasus UPB USB_PORT_CONTROL Ok
PORT_1='Off'
PORT_2='Off'
PORT_3='Off'
PORT_4='Off'
PORT_5='On'
PORT_6='Off'
2021-12-26T11:48:43: Client 0: queuing
2021-12-26T11:48:43: Client 0: sending msg copy 1 nq 2:


13.800000000000000711


0.5


7

2021-12-26T11:48:43: Client 0: sending msg copy 1 nq 1:


Off


Off


Off


Off


On


Off

2021-12-26T11:48:43: Driver indi_pegasus_upb: read setSwitchVector Pegasus UPB USB_HUB_CONTROL Idle

So I guess I do see indeed the proof that an IDLE light status is send in both case. Will close the bug, sorry again

[gnthibault]

Sorry, I cannot wrap my mind around this, it almost looks like there is an issue with the driver answering with a USB_HUB_CONTROL to a USB_PORT_CONTROL newswitch request.

Sample with erroneous behaviour:

2021-12-26T14:08:43: Client 0: read newSwitchVector Pegasus UPB USB_PORT_CONTROL
PORT_1='On'
PORT_2='Off'
PORT_3='Off'
PORT_4='Off'
PORT_5='On'
PORT_6='Off'
2021-12-26T14:08:43: Driver indi_pegasus_upb: queuing responsible for
2021-12-26T14:08:43: Client 5: queuing
2021-12-26T14:08:43: Client 5: sending msg copy 2 nq 1:


On


Off


Off


Off


On


Off

2021-12-26T14:08:43: Driver indi_pegasus_upb: sending msg copy 1 nq 1:


On


Off


Off


Off


On


Off

2021-12-26T14:08:43: Driver indi_pegasus_upb: read setSwitchVector Pegasus UPB USB_HUB_CONTROL Idle
INDI_ENABLED='On'
INDI_DISABLED='Off'

[gnthibault]

From what I understand from the code, there might be an issue at this exact line:

indi/drivers/auxiliary/pegasus_upb.cpp

Line 717 in ce74163

IDSetSwitch(&USBControlSP, nullptr);

I think that the
IDSetSwitch(&USBControlSP, nullptr);Should be replaced by a
IDSetSwitch(&USBControlV2SP, nullptr);

To me it looks like a copy paste kind of mistake, but I am not 100% sure, @knro can you please quickly check ? thank you in advance for your help

[gnthibault]

Ok problem fixed on my side thanks to this fix, I will try to create new PR

[gnthibault]

Note: this fix is enough to get me on track for my developments, but it is still not clear what is happening with USB_HUB_CONTROL on the UPB_V2 (I think somehow this was used in V1, but I don't understand what happened there, and what should be the expected behaviour for V2).
I don't need this other feature for now anyway, lets put it aside for now.

[gnthibault]

indi/drivers/auxiliary/pegasus_upb.cpp

Line 1227 in ce74163

bool PegasusUPB::setUSBPortEnabled(uint8_t port, bool enabled)

To me it looks like there is a weird behaviour when trying to set some values equal to the previous value held by the upbv2. If someone from Pegasus astro can give it a looks and crosscheck this code, that would be great.

[gnthibault]

Ok finally found the issues, fully fixed on my setup with this PR: #1581