Improvement with processing J2534 Connection #248
Conversation
kirya-dev
changed the title
udsoncan/connections.py
Outdated
| # Set up a J2534 interface using the DLL provided | ||
| self.interface = J2534(windll=windll, rxid=rxid, txid=txid, txFlags=txFlags) | ||
| try: | ||
| self.interface = J2534(windll=windll, rxid=rxid, txid=txid, txFlags=txFlags) |
There was a problem hiding this comment.
You changed an exception for another. What is the purpose?
The new exception doesn't say what was looked for, can you add it?
There was a problem hiding this comment.
Exactly FileNotFoundException controls it. No more information about inaccessible dll file in exception message.
In some times error message(depends locale) are different and we should control these.
In future we need create common udsoncan exceptions for common behaviour when dll or device inaccessible
There was a problem hiding this comment.
And we should controls in annotations which exceptions can be raised this library. We have one type RuntimeException for all situations… more stable application when it handles much more exception
|
Thank you for this. I added 2 comments. |
udsoncan/connections.py
Outdated
| # Open the interface (connect to the DLL) | ||
| self.result, self.devID = self.interface.PassThruOpen() | ||
| self.log_last_operation("PassThruOpen", with_raise=True) | ||
| except WindowsError as e: |
There was a problem hiding this comment.
Hmm this new exception causes CI to fail because CI runs on Linux and this is unknown.
We would probably need a more complex config that sets the OS to win32 when analyzing this file. I think this is doable with a .mypy.ini
| result = dllPassThruGetLastError(pErrorDescription) | ||
|
|
||
| return Error_ID(hex(result)), str(pErrorDescription.value) | ||
| return Error_ID(hex(result)), pErrorDescription.value.decode() |
There was a problem hiding this comment.
Previosly returns byte object like this b'Device is not opened'.
Now returns string Device is not opened.
|
Right now my PC is out of order. Will check to make CI happy next week |
|
I dont know why. Devices like OpenPort and Pulsar are not receiving messages correctly when no set RxStatus & ExtraDataindex stub value 0xCCCC_CCCC. |
|
I can't help. I never used J2534 :( |
|
I think it is some legacy. |
|
Do you plan other modifications? |
I work with this software every day on different devices and find different bugs 🐞. |
|
Can you apply this patch to your PR? That will make static analysis happy. |
|
Hmm. Container ubuntu:20.04 not same win32 😁 |
|
Definition of WindowsError in builtins.pyi I just replaced the exception type. |
|
Yeah. Will do a cleaner fix later. I think I can force mypy to assume windows through a mypy.ini |
2 participants
Points of improvement: