User/jvollmer/read can with submodule #2

BeckWangthumboon · 2025-03-05T01:42:51Z

Parse data

jrvollmer · 2025-03-05T03:00:04Z

read_can_messages.py

+signal_definitions = {
+    "0x20A": {"default": {"name": "ACC_IN", "data_type": "float"}},
+    "0x200": {"default": {"name": "MCU_ACC", "data_type": "float"}},
+    "0x201": {"default": {"name": "MCU_RGN_BRK", "data_type": "float"}},
+    "0x207": {
+        0: {"name": "MCU_DIR", "data_type": "boolean"},
+        1: {"name": "MCU_ECO", "data_type": "boolean"},
+        3: {"name": "PRK_BRK_TELEM", "data_type": "boolean"},
+    },
+    "0x202": {"default": {"name": "LV_12V_TELEM", "data_type": "float"}},
+    "0x203": {"default": {"name": "LV_5V_TELEM", "data_type": "float"}},
+    "0x204": {"default": {"name": "I_OUT_5V_TELEM", "data_type": "float"}},
+    "0x205": {"default": {"name": "I_IN_TELEM", "data_type": "float"}},
+    "0x209": {2: {"name": "MCU_MC_ON", "data_type": "boolean"}},
+    "0x206": {"default": {"name": "BRK_PRES_TELEM", "data_type": "float"}},
+}


jrvollmer · 2025-03-05T03:00:59Z

read_can_messages.py

+        print(type(id))
+        print(type(offset))


Can move to logging.debug() to avoid printing unless we're debugging

jrvollmer · 2025-03-05T03:02:59Z

read_can_messages.py

+        s = s[:-2]
+        # Add/Update the message data in the processed data format
+        p = processed.setdefault(id, {})
+        categories = SignalInfo(key, s[0], s[1], s[2], s[3], s[4], s[5])


Can we reference each element s[0] through s[1] by name for clarity here?

jrvollmer · 2025-03-05T03:03:55Z

read_can_messages.py

+        # Add/Update the message data in the processed data format
+        p = processed.setdefault(id, {})
+        categories = SignalInfo(key, s[0], s[1], s[2], s[3], s[4], s[5])
+        p[offset] = asdict(categories)


Let's keep this as a SignalInfo instance instead of casting to a dict

jrvollmer · 2025-03-05T03:05:18Z

read_can_messages.py

+                    print(
+                        f"New Message: ID={message_data['id']},Name={signal['name']} Value={float_value}, Time Stamp={message_data['timestamp']}"
+                    )
+
+        # handle booleans
+        else:
+            for key in signals:
+                signal = signals[key]
+                if signal["data_type"] == "boolean":
+                    byte_array = bytes(message.data)
+                    if not byte_array:
+                        logging.error(
+                            f"No data available for boolean signal in CAN ID {can_id}."
+                        )
+                        continue
+                    bit_offset = key
+                    bool_value = bool((byte_array[0] >> bit_offset) & 1)
+                    print(
+                        f"New Message: ID={message_data['id']},Name={signals[key]['name']} Value={bool_value}, Time Stamp={message_data['timestamp']}"
+                    )


Same as above. Can we change the print() calls to logging.debug() to avoid printing by default? If we do want to print by default, can we use logging.info()?

jrvollmer · 2025-03-05T03:06:24Z

read_can_messages.py

+
+
+logging.basicConfig(
+    level=logging.ERROR,


Can we decrease to logging.INFO? Generally, it's good to show info messages but filter out debug messages

jrvollmer · 2025-03-05T03:06:31Z

send_messages.py

+args = parser.parse_args()
+# add logging
+logging.basicConfig(
+    level=logging.ERROR,


Can we decrease to logging.INFO? Generally, it's good to show info messages but filter out debug messages

jrvollmer · 2025-03-05T03:07:29Z

send_messages.py

+# Allow user to input CAN channel
+parser = argparse.ArgumentParser(description="Specify the CAN channel.")
+# Define a positional argument for channel
+parser.add_argument("channel", type=str, help="CAN channel (e.g., can0, vcan0)")
+args = parser.parse_args()
+# add logging


Please move arg parsing inside if __name__ == '__main__': block to avoid argument parsing if imported

jrvollmer · 2025-03-05T03:08:36Z

send_messages.py

+                print(f"Byte out of range (0-255): {data_byte}")
+                bus.shutdown()
+                return
+            data_bytes.append(data_byte)
+    except ValueError:
+        logging.error("Invalid data byte input.")
+        bus.shutdown()
+        return
+
+    # Create the CAN message
+    msg = can.Message(
+        arbitration_id=arbitration_id, data=data_bytes, is_extended_id=False
+    )
+
+    # Send message using bus.send()
+    try:
+        bus.send(msg)
+        print(
+            f"Message sent successfully!\n Message Details: ID={msg.arbitration_id}, Data={msg.data}"
+        )
+    except can.CanError as e:
+        print(f"Message failed to send: {e}")


Same as above. Please use logging.debug/info/warning/error as appropriate instead of print

…format function

jrvollmer

Mostly nits

jrvollmer · 2025-03-16T16:57:22Z

read_can_messages.py

+            "timestamp": message.timestamp,
+        }
+
+        # loop through signal definitions to find can_id


nit: Not looping through

jrvollmer · 2025-03-16T17:04:34Z

read_can_messages.py

+            print(offset)
+            print(signals_info)


Can revert print statements or convert to debug logging statements for committed code

jrvollmer · 2025-03-16T17:07:38Z

read_can_messages.py

+            if data_type == "float":
+                if len(byte_array) < 4:
+                    logging.error(
+                        f"Insufficient data for float signal in CAN ID {can_id:0x}."
+                    )
+                    return
+                else:
+                    # Unpack the first 4 bytes as a little-endian float.
+                    float_value = struct.unpack("<f", byte_array[:4])[0]
+                    logging.debug(
+                        f"New Message: ID={can_id:0x},Name={signal_name} Value={float_value}, Time Stamp={message_data['timestamp']}"
+                    )
+            elif data_type == "boolean":
+                bool_value = bool((byte_array[0] >> offset) & 1)
+                logging.debug(
+                    f"New Message: ID={can_id:0x},Name={signal_name} Value={bool_value}, Time Stamp={message_data['timestamp']}"
+                )


Can we differentiate between all data types in the data format? Also, can we do a more strict check against the expected length in bytes for each value?

jrvollmer · 2025-03-16T17:08:49Z

read_can_messages.py

+    # set up listener
+    listener = MyListener()
+
+    # A Notifier runs in the background and listens for messages. When a new message arrives, it calls on_message in MyListener.


Suggested change

# A Notifier runs in the background and listens for messages. When a new message arrives, it calls on_message in MyListener.

# A Notifier runs in the background and listens for messages. When a new message arrives, it calls on_message_received in MyListener.

?

It's supposed to be on_message_received

jrvollmer · 2025-03-16T17:10:35Z

read_can_messages.py

+        parser.add_argument("channel", type=str, help="CAN channel (e.g., can0, vcan0)")
+        args = parser.parse_args()
+        transmit_can_message()


Can we split out transmitting and reading functionality for the test script (i.e. add another "tx" argument to enable transmitting (a) message(s))

BeckWangthumboon added 14 commits February 9, 2025 18:17

setup + created listener to recieve messages

a99dc8e

added loop to display messages

eac8e06

fix bugs

a09c3e7

transmit messages (GPT)

f8b5624

work in progress

b6ae409

added parsing for boolean and float values in messages.

27ad02f

fixed bug

732b7b2

test can hat

bfd9dbf

Add data format submodule

f0ab31d

TODO - add starter CAN info

dd60d39

Uppercase README.md

22c1610

TODO - Untested - Parse data format into a more parseable format

6f6c5eb

fixed parsing and add data class

8a74ec1

tested preprocess data format function

471126e

BeckWangthumboon requested a review from jrvollmer March 5, 2025 01:42

BeckWangthumboon self-assigned this Mar 5, 2025

jrvollmer requested changes Mar 5, 2025

View reviewed changes

BeckWangthumboon added 5 commits March 9, 2025 12:54

fixed parsing

45d78ce

just moved stuff aroun, deleted signal def and replaced it with data …

09a2d82

…format function

changed hex to int in on message_received

da22fe1

added hex to print message

a6923cc

fixed parsing again

74d1dfe

jrvollmer requested changes Mar 16, 2025

View reviewed changes

BeckWangthumboon added 2 commits March 16, 2025 12:15

created parse data method, moved dataclasses to seperate file

3700ca4

comments + logging change

2d5d0b7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User/jvollmer/read can with submodule #2

User/jvollmer/read can with submodule #2

BeckWangthumboon commented Mar 5, 2025

jrvollmer Mar 5, 2025

jrvollmer Mar 5, 2025

BeckWangthumboon Mar 9, 2025

jrvollmer Mar 5, 2025

BeckWangthumboon Mar 9, 2025

jrvollmer Mar 5, 2025

BeckWangthumboon Mar 9, 2025

jrvollmer Mar 5, 2025

BeckWangthumboon Mar 9, 2025

jrvollmer Mar 5, 2025

BeckWangthumboon Mar 9, 2025

jrvollmer Mar 5, 2025

BeckWangthumboon Mar 9, 2025

jrvollmer Mar 5, 2025

BeckWangthumboon Mar 9, 2025

jrvollmer Mar 5, 2025

BeckWangthumboon Mar 9, 2025

jrvollmer left a comment

jrvollmer Mar 16, 2025

BeckWangthumboon Mar 18, 2025

jrvollmer Mar 16, 2025

BeckWangthumboon Mar 18, 2025

jrvollmer Mar 16, 2025

jrvollmer Mar 16, 2025

BeckWangthumboon Mar 19, 2025

jrvollmer Mar 16, 2025

	# A Notifier runs in the background and listens for messages. When a new message arrives, it calls on_message in MyListener.
	# A Notifier runs in the background and listens for messages. When a new message arrives, it calls on_message_received in MyListener.



		logging.basicConfig(
		level=logging.ERROR,

User/jvollmer/read can with submodule #2

Are you sure you want to change the base?

User/jvollmer/read can with submodule #2

Conversation

BeckWangthumboon commented Mar 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrvollmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment