Add a new interface: dg interface #677

markCSDP · 2019-08-07T20:49:45Z

Here is a new interface for the DG BEACON tool from DG Technologies, Inc. (www.dgtech.com).
Note: we changed the version to 3.2.2, but you will need set it the correct version.

Update version to 3.1.1 and record in changelog

Release 3.2.0

Add notes to changelog.

Release v3.3.0

christiansandberg

You committed a pyc file instead of the source file. Is this intentional?

can/interfaces/dg/dg.py

doc/interfaces/dg.rst

codecov · 2019-08-13T20:25:57Z

Codecov Report

❗ No coverage uploaded for pull request base (develop@5ea5cfc). Click here to learn what that means.
The diff coverage is 1.95%.

@@            Coverage Diff            @@
##             develop    #677   +/-   ##
=========================================
  Coverage           ?   2.72%           
=========================================
  Files              ?      71           
  Lines              ?    6717           
  Branches           ?       0           
=========================================
  Hits               ?     183           
  Misses             ?    6534           
  Partials           ?       0

mikado98765 · 2019-08-13T20:48:52Z

We are aware we missed the 'dgBus' in can/interfaces/init.py. We'll fix that.

In `PcanBus.send()`, we initially set `msgType` based on all the flags of `msg`, including RTR. In the if/else for `self.fd`, we are incorrectly resetting it if rtr. We should not, and so we are no longer doing it.

zariiii9003 · 2019-08-17T12:51:20Z

Maybe it would make sense to export most of the code to an external package and import it as an optional dependency.
Right now your code is almost as big as all the other interfaces combined. Who is going to maintain that?

markCSDP · 2019-08-22T18:54:29Z

Maybe it would make sense to export most of the code to an external package and import it as an optional dependency.
Right now your code is almost as big as all the other interfaces combined. Who is going to maintain that?

You are correct. I will pull out the module for the BEACON tool into an external package.

…tional dependency.

… into develop

… develop

markCSDP · 2020-02-19T20:18:37Z

I am still having an issue getting this pull request to complete the testing. As requested by zariiii9003, I have removed the files. Should I just open a new pull request?

zariiii9003 · 2020-02-19T20:28:48Z

The log file says you have to fix the syntax in the setup.py:

Processing /home/travis/build/hardbyte/python-can
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-req-build-x978ibe6/setup.py", line 36
        <<<<<<< HEAD
         ^
    SyntaxError: invalid syntax

hardbyte

Thanks for the PR, everything is looking pretty good. I've left comments inline.
How did you get on splitting out a new Python library implementing the DG Gryphon Protocol?

I'd like to see tests for the utility functions _dict_to_msg and _list_to_int and their inverse.

There is still a .pyc file which should be removed.

hardbyte · 2020-11-16T01:40:12Z

test/test_dg.py

+# **********************************************************************
+# File Name: test_dg.py
+# Author(s): mohtake <mohtake@dgtech.com>
+# Target Project: python-can
+# Description:
+# Notes:
+# **********************************************************************
+#


Suggested change

# **********************************************************************

# File Name: test_dg.py

# Author(s): mohtake <mohtake@dgtech.com>

# Target Project: python-can

# Description:

# Notes:

# **********************************************************************

#

hardbyte · 2020-11-16T01:52:49Z

can/interfaces/dg/dg.py

+        self.ip = kwargs["ip"] if "ip" in kwargs else "localhost"
+        self.is_fd = kwargs["is_fd"] if "is_fd" in kwargs else False
+        self.bitrate = kwargs["bitrate"] if "bitrate" in kwargs else 500000
+        self.termination = kwargs["termination"] if "termination" in kwargs else True
+        self.pre_iso = kwargs["pre_iso"] if "pre_iso" in kwargs else False
+        self.data_bitrate = (
+            kwargs["data_bitrate"] if "data_bitrate" in kwargs else 2000000
+        )


It would be more idiomatic to have kwargs.get("ip", "localhost") for all of these.

hardbyte · 2020-11-16T01:53:27Z

can/interfaces/dg/dg.py

+#!/usr/bin/python
+#
+# ----------------------------------------------------------------------
+# pylint: disable=invalid-name
+# ----------------------------------------------------------------------
+#
+# **********************************************************************
+#
+# File Name: dg.py
+# Author(s): mohtake <mohtake@dgtech.com>
+# Target Project: python-can
+# Description:
+# Notes:
+# **********************************************************************
+#
+


Suggested change

#!/usr/bin/python

#

# ----------------------------------------------------------------------

# pylint: disable=invalid-name

# ----------------------------------------------------------------------

#

# **********************************************************************

#

# File Name: dg.py

# Author(s): mohtake <mohtake@dgtech.com>

# Target Project: python-can

# Description:

# Notes:

# **********************************************************************

#

hardbyte · 2020-11-16T01:53:54Z

can/interfaces/dg/dg.py

+# ----------------------------------------------------------------------
+import weakref
+from can import BusABC, Message
+from can.interfaces.dg.dg_gryphon_protocol import server_commands


I think the new dependency needs to be added and this import updated?

hardbyte · 2020-11-16T01:55:53Z

can/interfaces/dg/dg.py

+
+
+class DGBus(BusABC):
+    """ Beacon python-can backend API """


Any other backend specific notes aimed at developer users should go here. For instance I'd assume the messages are timestamped in hardware?

It also looks like you aren't supporting in hardware filtering? Is that a limitation of the gryphon protocol?

hardbyte · 2020-11-16T02:26:34Z

can/interfaces/dg/dg.py

+        self.beacon.CMD_CARD_SET_FILTER_MODE(
+            self.channel, self.beacon.FILTER_OFF_PASS_ALL
+        )


I assume there isn't any downside to calling this, and then a few lines down via the super calling set_filters?

hardbyte · 2020-11-16T02:29:54Z

can/interfaces/dg/dg.py

+            self.channel, self.beacon.FILTER_OFF_PASS_ALL
+        )
+        self.channel_info = ("dg channel '%s' on " + self.mode) % self.channel
+        super(DGBus, self).__init__(self.channel, None)


Something seems fishy here. Can you please explicitly deal with can_filters
As written I think they are just being dropped?

hardbyte · 2020-11-16T02:37:23Z

can/interfaces/dg/dg.py

+        for item in filters:
+            dataFil["filter_blocks"].append({})
+            dataFil["filter_blocks"][counter]["byte_offset"] = 0
+            dataFil["filter_blocks"][counter][
+                "data_type"
+            ] = self.beacon.FILTER_DATA_TYPE_HEADER
+            dataFil["filter_blocks"][counter]["operator"] = self.beacon.BIT_FIELD_CHECK
+            dataFil["filter_blocks"][counter]["mask"] = self._int_to_list(
+                item["can_mask"]
+            )
+            dataFil["filter_blocks"][counter]["pattern"] = self._int_to_list(
+                item["can_id"]
+            )
+            counter += 1


Suggested change

for item in filters:

dataFil["filter_blocks"].append({})

dataFil["filter_blocks"][counter]["byte_offset"] = 0

dataFil["filter_blocks"][counter][

"data_type"

] = self.beacon.FILTER_DATA_TYPE_HEADER

dataFil["filter_blocks"][counter]["operator"] = self.beacon.BIT_FIELD_CHECK

dataFil["filter_blocks"][counter]["mask"] = self._int_to_list(

item["can_mask"]

)

dataFil["filter_blocks"][counter]["pattern"] = self._int_to_list(

item["can_id"]

)

counter += 1

for item in filters:

dataFil["filter_blocks"].append({

"byte_offset": 0,

"data_type": self.beacon.FILTER_DATA_TYPE_HEADER,

"operator": self.beacon.BIT_FIELD_CHECK,

"mask": self._int_to_list(item["can_mask"]),

"pattern": self._int_to_list(item["can_id"])

})

hardbyte · 2020-11-16T02:39:39Z

can/interfaces/dg/dg.py

+        )
+
+    def stop(self):
+        self.beaconref().CMD_SCHED_KILL_TX(self.channel, self.idIn)


Suggested change

self.beaconref().CMD_SCHED_KILL_TX(self.channel, self.idIn)

self.beaconref().CMD_SCHED_KILL_TX(self.channel, self.idIn)

felixdivo · 2020-11-23T11:26:46Z

doc/interfaces/dg.rst

+================
+
+Interface to `DG Technologies <http://dgtech.com>`__ Beacon
+


Are there any extra drivers required for this? What platforms (operating systems) does it run on?

felixdivo · 2020-11-23T11:30:56Z

doc/interfaces/dg.rst

+The ``DGBus`` class implements several python-can methods::
+
+    send, send_periodic, recv, set_filters, flush_tx_buffer, _detect_available_configs, shutdown


I think there is no need to mention the methods which definitely need to be implemented anyways. It raises the question if other interfaces do it even if they do not mention it. I'd suggest including these:

Suggested change

The ``DGBus`` class implements several python-can methods::

send, send_periodic, recv, set_filters, flush_tx_buffer, _detect_available_configs, shutdown

The ``DGBus`` class implements several optional methods: ``send_periodic`` and ``set_filters`` within the driver, as well as ``flush_tx_buffer`` and ``_detect_available_configs``.

felixdivo · 2020-11-23T11:32:30Z

Cool mocking tests!

felixdivo · 2021-04-13T16:19:07Z

@markcdg Do you plan on working further on this PR? It looks like a neat addition to the library!

felixdivo · 2021-09-28T07:23:15Z

I opened #1132 to track this topic and will close this one as there is apparently no development effort going on.

hardbyte and others added 13 commits February 26, 2019 18:31

Merge pull request hardbyte#524 from hardbyte/release-3.1.1

2f6830d

Update version to 3.1.1 and record in changelog

Merge pull request hardbyte#580 from hardbyte/release-3.2.0

d5dd7fe

Release 3.2.0

update wheel package before deploy

2a30f25

Minor version bump and doc update

ad3f830

Minor version bump to v3.2.1

a91ca25

Update changelog for v3.2.1

667278e

Update version to v3.3.0

a0f0ea7

Add notes to changelog.

Merge pull request hardbyte#629

07d1a18

Release v3.3.0

Backport pytest-runner fix from hardbyte#633

591967c

Bump version to 3.3.1

1f0fb8b

Add a new interface: dg interface

f90f6be

Add a new interface: dg interface including server_commands file

692d082

Add a new interface: dg interface removed comments

a1883e5

christiansandberg requested changes Aug 8, 2019

View reviewed changes

markCSDP and others added 3 commits August 8, 2019 14:21

Merge branch 'develop' into develop

91a56d3

Merge remote-tracking branch 'upstream/develop' into develop

fbe0d79

Updates from comments from first attempt at doing the pull request

f25d5b9

markCSDP and others added 10 commits August 16, 2019 14:02

Merge remote-tracking branch 'upstream/develop' into develop

7dddd29

Changed from dgBus to DGBus

182f4ad

adding server_commands.py file, fixed detect()

70fc573

fix for test of thread-safe sched

fb27c14

removed failing test

d44acd8

removed failing test

c1b58a8

fixed pylint and black errors

2534898

ran Python3.6 black on dg.py

efc960b

Do not incorrectly reset CANMsg.MSGTYPE on remote frame.

1d4c6a7

In `PcanBus.send()`, we initially set `msgType` based on all the flags of `msg`, including RTR. In the if/else for `self.fd`, we are incorrectly resetting it if rtr. We should not, and so we are no longer doing it.

Bump version to 3.3.2

29a235b

markCSDP and others added 5 commits September 19, 2019 10:19

As requested, removed the Gryphon Protocol Python, put as external op…

6aa844e

…tional dependency.

Merge branch 'develop' into develop

9ae917d

Merge remote-tracking branch 'upstream/master' into develop

cc55956

Merge branch 'develop' of https://github.com/DG-Technologies/python-can…

5ea5cfc

… into develop

Merge branch 'develop' of https://github.com/hardbyte/python-can into…

a8c8212

… develop

markCSDP added 3 commits February 20, 2020 10:58

fixed issues caused by merge

a2c93c7

fixes from merge issue

c5fbcba

syntax correction, from fixes to merge

f4e8a3e

mergify bot requested a review from hardbyte November 16, 2020 00:19

Merge branch 'develop' into develop

ac79af0

hardbyte requested changes Nov 16, 2020

View reviewed changes

felixdivo reviewed Nov 23, 2020

View reviewed changes

felixdivo added the work-in-progress label May 11, 2021

felixdivo mentioned this pull request Sep 28, 2021

Add support for devices from "DG Technologies" #1132

Open

felixdivo closed this Sep 28, 2021

	self.beaconref().CMD_SCHED_KILL_TX(self.channel, self.idIn)
	self.beaconref().CMD_SCHED_KILL_TX(self.channel, self.idIn)

		================

		Interface to `DG Technologies <http://dgtech.com>`__ Beacon

		The ``DGBus`` class implements several python-can methods::

		send, send_periodic, recv, set_filters, flush_tx_buffer, _detect_available_configs, shutdown

Add a new interface: dg interface #677

Add a new interface: dg interface #677

Uh oh!

Conversation

markCSDP commented Aug 7, 2019

Uh oh!

christiansandberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Aug 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mikado98765 commented Aug 13, 2019

Uh oh!

zariiii9003 commented Aug 17, 2019

Uh oh!

markCSDP commented Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markCSDP commented Feb 19, 2020

Uh oh!

zariiii9003 commented Feb 19, 2020

Uh oh!

hardbyte left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixdivo Nov 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixdivo commented Nov 23, 2020

Uh oh!

felixdivo commented Apr 13, 2021

Uh oh!

felixdivo commented Sep 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov bot commented Aug 13, 2019 •

edited

Loading

markCSDP commented Aug 22, 2019 •

edited

Loading

felixdivo Nov 23, 2020 •

edited

Loading