Fixed Slcan fileno raising an exception on windows

[bmeisels]

Slcan currently raises an exception when used with 'can.Notifier' on windows.

'can.Notifier.add_bus' calls 'bus.fileno' which calls 'serialPortOrig.fileno' which raises an exception on windows as it is not implemented.

The fix catches the exception raised by calling fileno instead of checking if it exists(as it can exist and still raise an error).

[codecov]

Codecov Report

Merging #760 into develop will decrease coverage by 0.60%.
The diff coverage is 25.00%.

@@             Coverage Diff             @@
##           develop     #760      +/-   ##
===========================================
- Coverage    70.39%   69.79%   -0.61%     
===========================================
  Files           70       70              
  Lines         6840     6488     -352     
===========================================
- Hits          4815     4528     -287     
+ Misses        2025     1960      -65     

[bessman]

Better specify the error type. Naked excepts can hide bugs.

[bmeisels]

@felixdivo @bessman I made the changes. What do you think?

[bessman]

Can NotImplementedError or AttributeError actually be raised here? If not we shouldn't try to catch them. Based on the old code it looks like at least AttributeError could be raised. @christiansandberg, do you remember the reason for the hasattr check?

[christiansandberg]

That was probably a mistake from my end. I saw that pyserial did not implement the fileno() method but forgot to check if it was inherited instead.

[bessman]

In that case, if the only exception that has actually been observed in the wild here is UnsupportedOperation, I suggest only trying to catch that. If it turns out other exceptions (that we want to catch) can be raised here, add them later.

[bmeisels]

Changed to only catch io.UnsupportedOperation.

[christiansandberg]

I think an even better approach would be to implement the fileno() method in BusABC which would raise UnsupportedOperation. The slcan implementation would not need to handle the exception at all, but the Notifier would do that instead. Returning -1 instead of raising an exception was not very Pythonic I admit.

[bmeisels]

@christiansandberg I like the general idea.
I think BusABC.fileno should raise NotImplementedError to be similar to other optional method in BusABC.

[bmeisels]

Sorry for the inactivity. I thought we were leaning towards the solution offered by @christiansandberg .

[hardbyte]

Ahh right you are. I've opened #877 if you'd like to check it out