On Sun, Mar 8, 2020 at 7:13 PM Gerhard Sittig <gerhard.sit...@gmx.net> wrote:
> On Fri, 2020-03-06 at 23:06 +0000, Andreas Sandberg wrote: > > > > [ ... ] > > > > Related to these pull requests, there is a workaround for a Bluetooth > issue > > that I encountered when connecting to my UM24C. The workaround [6] adds a > > retry if connect() returns EBUSY. There was a discussion about this on > the > > list a while back. > > > > [ ... ] > > > > [6] https://github.com/sigrokproject/libsigrok/pull/44 > > The discussion did not reveal the time span which was involved. > That's when I assumed the typical issue of half-closed sockets. > Your patch suggests it's fractions of a second instead. Which is > rather different. > > Only had a cursory look. The change is to repeat the connect() > attempt up to three times with 100ms in between? Looks plausible > to me. Would not kick in for already-working setups, only helps > avoid occassional failure in edge cases. Good. > Exactly, that's the intention of the patch. > Would be nice to check this in more setups. Who runs serially > attached devices in combination with e.g. HC05 "cables", or wants > to? And can help test the change, to increase coverage? > I'm afraid I don't have any such cable to test with. However, I can't see any reason for this change to break other bluetooth devices. Scanning my UM device, where the first connection succeeds works, so things should still work for well-behaved devices. > Minor style nits, some of it a matter of taste: I'd rather count > retries backwards. Better communicates when they are exhausted. > Check for leaks. Does the bottom perror(3) case lack a close(2) > call? Is shutdown(2) required before close(2) since there is no > other layer in play for the error paths? Or is shutdown(2) not > needed because connect(2) did not succeed yet? What about the > EBUSY case? Thanks for pointing out the FD leak, I completely missed that, I have updated the patch to: 1) add the missing close(2), and 2) add an error message if the retries are exhausted. My understanding is that shtudown(2) is only needed for connected sockets. I.e., it's not needed if connect(2) fails regardless of the reason. As for counting backwards, I'd rather avoid that pattern since there are common corner cases where people inadvertently create infinite loops when combined with unsigned variables. > Won't comment on the UM/TC part due to lack of knowledge. > That's fine. Do you happen to know how to speed up the process? The changes have been sitting around as a pull request for more than a month now and I posted a link to the branch more than a year ago on IRC (Uwe even confirmed that he received it). Thanks, Andreas
_______________________________________________ sigrok-devel mailing list sigrok-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sigrok-devel
