ni / nixnet-python

NI-XNET Python API
https://nixnet.readthedocs.io/en/latest/
MIT License
25 stars 22 forks source link

CAN examples which use floating point round to the nearest integer #259

Open chlc3d opened 6 years ago

chlc3d commented 6 years ago

Reproduction steps:

  1. Connect two CAN ports named CAN1 and CAN2
  2. go to nixnet_examples and run can_signal_conversion.py or can_signal_single_point_io.py
  3. Insert a non-integer number when prompted
  4. Note that the console says "the same values should be received", but the output is rounded to the nearest integer.

Installed packages and version

nixnet 0.2.1 pip 9.0.3 setuptools 39.0.1 six 1.11.0

jashnani commented 6 years ago

It's rounding values to nearest int because the datatype on the Signal object used by those examples (CANEventSignal1) is a signed int. You can confirm this by opening the database in an editor and checking the datatype value.

We should consider either modifying the database or the example to better communicate the point. Slight preference for modifying the database so we can show that float values are also supported for Signals.

d-bohls commented 6 years ago

If we modify the NIXNET_example database that ships with the NI-XNET driver, then the example won't work with older XNET drivers installed. We should probably add another database in "nixnet-python\nixnet_examples\databases" that has the structure we want for our example.

lilesj commented 6 years ago

The LabVIEW shipping examples work this way and also round the output.

I want to avoid having a special version of the example database specifically for Python. And since there are no signals in the NIXNET_example db that have the IEEE Float data type we won't be able to avoid the rounding.

The primary purpose of the examples is to demonstrate how to use the API and verify that the hardware is working. I suggest that we add additional text to the examples to better explain what input the example is requesting, what the example is doing, and what the expected output is. For example we could say something along these lines: This example will transmit both CANEventSignal1(signed int) and CANEventSignal2(signed int) from one interface to the other. Please enter two signal values to transmit [float, float]: Values received (note the rounding that occurs due to the signal data type) CANEventSignal1: xxx CANEventSignal2: xxx

d-bohls commented 6 years ago

I want to avoid having a special version of the example database specifically for Python.

Agreed, I don't think python should have its own special version of NIXNET_example. My thinking was that if we do in fact need floats (which are not contained anywhere in the NIXNET_example database) then we could add a new database with a cluster containing float signals for use with that example. Doing this would not be unprecedented. We already have on github a database named "custom_database.dbc" which only the "programmatic_database_usage.py" python uses.

Two of the python examples ask for floats; the rest ask for ints. I don't like the idea of doing rounding from float to int unless it contributes directly to conveying the example's intended purpose. If floats/rounding aren't needed, perhaps we should just stop asking for floats and modify the two float examples to request ints instead.

lilesj commented 6 years ago

I am comfortable modifying the examples to request integers.

jashnani commented 6 years ago

We further discussed this in the grooming meeting and decided to add a comment in code explaining why we're coercing to type int.