Add CAN and LIN database creation examples by d-bohls · Pull Request #219 · ni/nixnet-python

d-bohls · 2018-01-22T23:48:38Z

Fixes #218

This contribution adheres to CONTRIBUTING.md.
New tests have been created for any new features or regression tests for bugfixes.
tox successfully runs, including unit tests and style checks (see CONTRIBUTING.md).

coveralls · 2018-01-22T23:56:15Z

Coverage increased (+2.5%) to 67.818% when pulling 024bacc on d-bohls:databaseExamples into 3f72c1f on ni:master.

epage

Overall, good job! Also, the commit history is very clean!

Some minor comments on conventional commit style

chore(API): Add enum for LinProtocolVersion
- I'd classify chore as work that has no user impact, like changing line endings, comment styles, etc. By default, chore does not show up in the changelog with the tooling I at least use
  - fix(examples): Fix typo in comment is a good example of a chore.
- I'd put the focus on this more on the major impact. LinProtocolVersion by itself isn't too helpful.
- So maybe something like fix(API): Expose Ecu.lin_protocol_ver as the new LinProtocolVer enum
chore(API): Change LinSchedEntry.frames to Frames
- This would be a fix like LinProtocolVersion
renaming user-visible API calls, changing data types on user-visible API calls, etc should be noted in a BREAKING CHANGES section in the commit message.
- Now whether we bump 0.x (pre-release's major) for this sake is philosophical (was this API more of an easter egg / early access and doesn't need to be taken into account with semver?)

epage · 2018-01-22T23:51:41Z

nixnet/_enums.py

    CRC = _cconsts.NX_LIN_LAST_ERR_CODE_CRC


+class LinProtocolVersion(enum.Enum):


Weren't we talking about abbreviating Version here like in the constant name?

I did change the members. I'll change the class name to LinProtocolVer too.

epage · 2018-01-22T23:51:57Z

nixnet/_cprops.py

 def get_database_ref_array(ref, prop_id):
    # type: (int, int) -> typing.Iterable[int]
-    value_size = _funcs.nx_get_property_size(ref, prop_id)
+    value_size = _funcs.nxdb_get_property_size(ref, prop_id)


Oops, copy/paste bug on my part. Sorry you had to deal with it

epage · 2018-01-22T23:53:14Z

nixnet/db/_linsched_entry.py


 from nixnet import _props
 from nixnet import constants
+from nixnet.db import _frame


Is linsched_entry a descendant of frame?

I recommend all back references do the imports in the functions for consistency to help avoid problems

I don't think so. The hierarchy looks like this:

Cluster
___Frame
___LinSched
______LinSchedEntry

So Frame is a sibling of LinSchedEntry's parent.

epage · 2018-01-22T23:54:58Z

nixnet_examples/lin_dynamic_database_creation.py

+from nixnet import db
+
+
+def main():


Each example

Should have a unit test

Should be included in the documentation

epage · 2018-01-22T23:55:04Z

nixnet_examples/can_dynamic_database_creation.py

+from nixnet import db
+
+
+def main():


Each example

Should have a unit test

Should be included in the documentation

epage · 2018-01-22T23:56:00Z

nixnet/db/_linsched_entry.py

    @property
    def frames(self):
-        return _props.get_lin_sched_entry_frames(self._handle)
+        # type: () -> typing.Iterable[_frame.Frame]


From travis: nixnet/db/_linsched_entry.py:58: error: Name 'typing' is not defined

You need to import typing. See some of the other files for how to do it without causing flake8 problems

epage · 2018-01-22T23:56:10Z

nixnet/db/_linsched_entry.py

    @frames.setter
    def frames(self, value):
-        _props.set_lin_sched_entry_frames(self._handle, value)
+        # type: (typing.Iterable[_frame.Frame]) -> None


ditto about mypy error from travis

In CONTRIBUTING.rst, the tox integration command that includes LIN should use output interface LIN2 and not CAN2.

Use nxdb_get_property_size instead of nx_get_property_size when getting the size of database properties.

Fixes ni#218 BREAKING CHANGE: Rename `Db.cluster` to `Db.clusters`

BREAKING CHANGE: Modify LinSchedEntry.frames to get/set a list of Frame objects instead of a list of db refs.

BREAKING CHANGE: Add enum LinProtocolVer and modify Ecu.lin_protocol_ver to use it.

Add database creation examples for CAN and LIN. Add documentation and testing for the examples.

epage · 2018-01-26T21:43:06Z

Well, I see nothing wrong and I happen to still have write access. I guess I'll merge this :)

epage suggested changes Jan 23, 2018

View reviewed changes

d-bohls added 8 commits January 26, 2018 15:18

chore: Add .mypy_cache folder to .gitignore

87436c7

docs: Fix tox integration command in CONTRIBUTING.rst

f617740

In CONTRIBUTING.rst, the tox integration command that includes LIN should use output interface LIN2 and not CAN2.

fix(examples): Fix typo in example comment

e43b498

fix(API): Use nxdb_get_property_size for db props

b5c5f9a

Use nxdb_get_property_size instead of nx_get_property_size when getting the size of database properties.

fix(API): Db.cluster should be Db.clusters

57fb0cf

Fixes ni#218 BREAKING CHANGE: Rename `Db.cluster` to `Db.clusters`

fix(API): Change LinSchedEntry.frames to Frames

855b783

BREAKING CHANGE: Modify LinSchedEntry.frames to get/set a list of Frame objects instead of a list of db refs.

fix(API): Change type of Ecu.lin_protocol_ver

02dac70

BREAKING CHANGE: Add enum LinProtocolVer and modify Ecu.lin_protocol_ver to use it.

feat(examples): Add database creation examples

024bacc

Add database creation examples for CAN and LIN. Add documentation and testing for the examples.

d-bohls force-pushed the databaseExamples branch from 56766d1 to 024bacc Compare January 26, 2018 21:19

epage approved these changes Jan 26, 2018

View reviewed changes

epage merged commit cdef471 into ni:master Jan 26, 2018

d-bohls deleted the databaseExamples branch January 26, 2018 22:01

d-bohls mentioned this pull request Feb 17, 2018

Ecu.lin_protocol_ver should return an enum #54

Closed

d-bohls self-assigned this Mar 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CAN and LIN database creation examples#219

Add CAN and LIN database creation examples#219
epage merged 8 commits intoni:masterfrom
d-bohls:databaseExamples

d-bohls commented Jan 22, 2018

Uh oh!

coveralls commented Jan 22, 2018 •

edited

Loading

Uh oh!

epage left a comment

Uh oh!

epage Jan 22, 2018

Uh oh!

d-bohls Jan 26, 2018

Uh oh!

epage Jan 22, 2018

Uh oh!

epage Jan 22, 2018

Uh oh!

d-bohls Jan 26, 2018 •

edited

Loading

Uh oh!

epage Jan 22, 2018

Uh oh!

epage Jan 22, 2018

Uh oh!

epage Jan 22, 2018

Uh oh!

epage Jan 22, 2018

Uh oh!

epage commented Jan 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		CRC = _cconsts.NX_LIN_LAST_ERR_CODE_CRC


		class LinProtocolVersion(enum.Enum):

Conversation

d-bohls commented Jan 22, 2018

Uh oh!

coveralls commented Jan 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

epage 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!

d-bohls Jan 26, 2018 • 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!

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!

epage commented Jan 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented Jan 22, 2018 •

edited

Loading

d-bohls Jan 26, 2018 •

edited

Loading