introducing canal interface#161
Closed
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
* almost just a copy of the usb2can module which is badly named * the usb2can module is for now left unchanged but should be changed to use the new CANAL interface * TODO serial_selector update...
* autodetect device ID failed * CanalOpen failed
grodansparadis commented May 9, 2017 via email
Applause! :) …On 09/05/17 14:17, krixkrix wrote: I have added a CANAL interface for python-can. It is more or less a copy of the usb2can interface code, which wraps the canal API the same way. The usb2can interface should IMHO be called "canal" but I did not want to interfere with that interface, so I have created a new one. I also extended the canal wrapper a bit with some error checking. The canal interface has been tested with a Lawicel CANUSB adapter. http://www.vscp.org/docs/vscpd/doku.php?id=level1_driver_canusb This is my first pull request ever, so feel free to teach me how I should have done. ------------------------------------------------------------------------ You can view, comment on, or merge this pull request online at: #161 Commit Summary * Introduce CANAL interface * make the serial device autodetect work * Canal interface: Raise CanError on various errors: * Add an example file File Changes * *M* can/interface.py <https://github.com/hardbyte/python-can/pull/161/files#diff-0> (3) * *M* can/interfaces/__init__.py <https://github.com/hardbyte/python-can/pull/161/files#diff-1> (2) * *A* can/interfaces/canal/__init__.py <https://github.com/hardbyte/python-can/pull/161/files#diff-2> (2) * *A* can/interfaces/canal/canalInterface.py <https://github.com/hardbyte/python-can/pull/161/files#diff-3> (183) * *A* can/interfaces/canal/canal_wrapper.py <https://github.com/hardbyte/python-can/pull/161/files#diff-4> (151) * *A* can/interfaces/canal/serial_selector.py <https://github.com/hardbyte/python-can/pull/161/files#diff-5> (39) * *A* examples/canal_demo.py <https://github.com/hardbyte/python-can/pull/161/files#diff-6> (50) Patch Links: * https://github.com/hardbyte/python-can/pull/161.patch * https://github.com/hardbyte/python-can/pull/161.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#161>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABS8I-VT5CKuh-Qg_fm2IHKVspvUhnI5ks5r4FlsgaJpZM4NVOry>. -- Stay Hungry - Stay Foolish! Åke Hedman Brattbergavägen 17, 82050 LOS, SWEDEN http://www.vscp.orghttp://www.akehedman.se |
Collaborator
christiansandberg left a comment
christiansandberg left a comment There was a problem hiding this comment.
Looks good. But it's bad practice for libraries to hide serious errors, so I suggest to add some more exceptions and let the application catch them and decide what to do. Preferably create a new exception class inheriting from CanError which can present it with an error description based on the status code.
| if timeout: | ||
| self.can.blocking_send(self.handle, byref(tx), int(timeout * 1000)) | ||
| else: | ||
| self.can.send(self.handle, byref(tx)) |
Collaborator
There was a problem hiding this comment.
Check result and raise exception if error.
| # CANAL_ERROR_RCV_EMPTY or CANAL_ERROR_TIMEOUT | ||
| rx = None | ||
| else: | ||
| log.error('Canal Error %s', status) |
Collaborator
There was a problem hiding this comment.
Raise exception instead.
| def shutdown(self): | ||
| """Shut down the device safely""" | ||
| status = self.can.close(self.handle) |
Collaborator
There was a problem hiding this comment.
Check status and raise exception if error.
| for i in range(length): | ||
| messagetx.data[i] = msg.data[i] | ||
| messagetx.flags = 80000000 |
Collaborator
There was a problem hiding this comment.
Should be 0x80000000.
| REMOTE_FRAME = bool(messagerx.flags & IS_REMOTE_FRAME) | ||
| ERROR_FRAME = bool(messagerx.flags & IS_ERROR_FRAME) | ||
| msgrx = Message(timestamp=messagerx.timestamp, |
Collaborator
There was a problem hiding this comment.
This should be in seconds, so divide with 1000.0.
Collaborator
felixdivo commented Dec 17, 2017
@krixkrix Can you put in these few changes? I would do it, but I do not know how to branch off a PR and mere it into that one again. |
hardbyte requested changes Feb 9, 2018
Owner
hardbyte left a comment
hardbyte left a comment There was a problem hiding this comment.
Happy to merge after the changes Christian pointed out have been made.
Closed
Collaborator
felixdivo commented Feb 10, 2018
I rebased it, added the requested changes and created an new PR in #245. |
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have added a CANAL interface for python-can.
It is more or less a copy of the usb2can interface code, which wraps the canal API the same way.
The usb2can interface should IMHO be called "canal" but I did not want to interfere with that interface, so I have created a new one.
I also extended the canal wrapper a bit with some error checking.
The canal interface has been tested with a Lawicel CANUSB adapter.
http://www.vscp.org/docs/vscpd/doku.php?id=level1_driver_canusb
This is my first pull request ever, so feel free to teach me how I should have done.