XHC WHB04B development?

The crashes I got was after removing the assert() in setupAsynchTransfer().
When that condition happened, the code waited three seconds and tried to to re-establish
communication. I then got a segv crash from within libusb  forum.linuxcnc.org/38-general-linuxcnc-q...ment?start=90#343894
I tried on two different computers and got the same crashes on both. Both debian 12.13, gcc 12.
When i upgraded to debian 13 and gcc 14, there are no more crashes.
I also don't get that debug message from libusb about destroying device.
"Only" changing the gcc compiler version did change things to the better.

My way of get the xhc component to fail is to increase the loops to 100 in hal.h,
start G1X-200F100 in MDI and once that is slowly running, I spin the wheel very fast in the Z direction,
or any direction, but Z is safe.
With default value of loops it takes some tries to get it to fail, and with loops 10 it is almost impossible to get it to fail.
After it fails it usually restarts after a few seconds. Unless the libusb crashes it with segv as it did with gcc 12.

I did try your component yesterday for a short time and it never failed. 

I sat for some fifteen minutes and tried my best to make the xhc fail. The errorhandling branch.
It works fine. The movement I used to make it fail is now blocked. And for the rest I just works.
I tried things that takes a bit of time like switch screen that used to be a problem.
Not a proof it always works, but I am unable to make it fail.

 

I tested the errorhandling branch by modifying the requestManaulMode back to the original. Then I can get the dongle to disconnect reliably. While I maybe did not do exhaustive testing, the dongle reconnected successfully every time I generated a disconnect. So at least based on this your code is working as expected.

Also with the original code, the pendant would reconnect for me if I simply removed the assert statement. I know that no "restart" is triggered from that alone. But because the USB device has been disconnected, the libusb_control_transfer will fail just a few moment later, and that will trigger the restart. For Hakan it didn't reconnect, but that was because it segfaulted in libusb. Anyway, now the assert statement is gone and there is some proper handling of the libusb_submit_transfer errors.

While you are there in the setupAsyncTransfer function, the libusb_fill_bulk_transfer() should really be a libusb_fill_interrupt_transfer(). If you do a lsusb -vvv, you see that endpoint 1 IN is really an interrupt endpoint: The bInterval means it should be polled every 2 ms (for a full speed device, as this is). I don't think that is guaranteed by a bulk transfer. And the interrupt transfers should also be given higher priority on the bus than bulk. I'm not saying that using bulk transfer causes issues. But because we have also had disconnect that are in my opinion not related to mode switching, it's at least possible that using interrupt transfers instead of bulk could improve the situation. And it's the right transfer type to use...

Thanks for testing!

There might also have been a bug in libusb. This kind of error handling is always difficult to test when you don't have errors.

I did not go that deep into USB. But your explanation sounds reasonable. I changed it to interrupt. It did not make a difference for me but rather correct than wrong but still working, I pushed a new commit. Feel free to test if you did not test that anyway already.

Note: There are also other usleep() in hal.cc. So they might also give issues. However, fixing that is a lot of effort. I thing it's easier to write it new from scratch using parts of the existing code. But if it works now, that's fine anyway...

Tested the latest changes, and everything works as expected. Thank you!

Yeah. I also grepped for usleep at one point, but didn't really bother trying to figure out if it will cause issues elsewhere. If you go back to some of the first posts, you see I also considered writing a completely new component. Part of the reason for that was to have a state model and avoid these blocking sleep calls. The other part was that I think that the level of abstractions used is bordering on ridiculous for the application. The xhc-hb04.cc component has one order of magnitude less lines of code and does more or less the same thing. But now that you are fixing things, and with my own firmware, I personally don't see a need for a completely new component at this time.

Hannes wrote:

@rodw: Jogging in manual mode is fine. My machine can not jog in teleop mode due to I have two Y axis servo motors. The only thing the WHB component does after my PR is:
- Change to teleop mode if not homed (which fails on my machine but on others, it will do that) -> You can jog before homing any special action
- Change to manual mode if homed AND program is idle -> You can jog after homing without any special action

In this case its far better to disable motion before homing with a simple change to the ini file. I still have no idea why people want to move or jog before homing. None of my machines allow this! And yes, one was a XYYZ gantry plasma table that jogged with a pendant perfectly!