From 695b74d80ee7e53f859e699c6d46290b553007d2 Mon Sep 17 00:00:00 2001
From: deanlee <deanlee3@gmail.com>
Date: Thu, 6 Jun 2024 15:44:23 +0800
Subject: [PATCH] Return empty values and log an error for invalid addresses.

log error for undefined signal

add comment

use LOGE
---
 can/packer.cc      | 10 ++++++++--
 can/packer_pyx.pyx | 22 ++++++++++++++--------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/can/packer.cc b/can/packer.cc
index d8f9dcb82b..9e6c01dc72 100644
--- a/can/packer.cc
+++ b/can/packer.cc
@@ -41,7 +41,13 @@ CANPacker::CANPacker(const std::string& dbc_name) {
 }
 
 std::vector<uint8_t> CANPacker::pack(uint32_t address, const std::vector<SignalPackValue> &signals) {
-  std::vector<uint8_t> ret(dbc->addr_to_msg.at(address)->size, 0);
+  auto msg_it = dbc->addr_to_msg.find(address);
+  if (msg_it == dbc->addr_to_msg.end()) {
+    LOGE("invalid address %d\n", address);
+    return {};
+  }
+
+  std::vector<uint8_t> ret(msg_it->second->size, 0);
 
   // set all values for all given signal/value pairs
   bool counter_set = false;
@@ -49,7 +55,7 @@ std::vector<uint8_t> CANPacker::pack(uint32_t address, const std::vector<SignalP
     auto sig_it = signal_lookup.find(std::make_pair(address, sigval.name));
     if (sig_it == signal_lookup.end()) {
       // TODO: do something more here. invalid flag like CANParser?
-      WARN("undefined signal %s - %d\n", sigval.name.c_str(), address);
+      LOGE("undefined signal %s - %d\n", sigval.name.c_str(), address);
       continue;
     }
     const auto &sig = sig_it->second;
diff --git a/can/packer_pyx.pyx b/can/packer_pyx.pyx
index 78bd3c0866..bf301c47ae 100644
--- a/can/packer_pyx.pyx
+++ b/can/packer_pyx.pyx
@@ -1,7 +1,7 @@
 # distutils: language = c++
 # cython: c_string_encoding=ascii, language_level=3
 
-from libc.stdint cimport uint8_t
+from libc.stdint cimport uint8_t, uint32_t
 from libcpp.vector cimport vector
 
 from .common cimport CANPacker as cpp_CANPacker
@@ -37,11 +37,17 @@ cdef class CANPacker:
     return self.packer.pack(addr, values_thing)
 
   cpdef make_can_msg(self, name_or_addr, bus, values):
+    cdef uint32_t address = 0
     cdef const Msg* m
-    if isinstance(name_or_addr, int):
-      m = self.dbc.addr_to_msg.at(name_or_addr)
-    else:
-      m = self.dbc.name_to_msg.at(name_or_addr.encode("utf8"))
-
-    cdef vector[uint8_t] val = self.pack(m.address, values)
-    return [m.address, 0, (<char *>&val[0])[:val.size()], bus]
+    try:
+      if isinstance(name_or_addr, int):
+        m = self.dbc.addr_to_msg.at(name_or_addr)
+      else:
+        m = self.dbc.name_to_msg.at(name_or_addr.encode("utf8"))
+      address = m.address
+    except IndexError:
+      # The C++ pack function will log an error message for invalid addresses
+      pass
+
+    cdef vector[uint8_t] val = self.pack(address, values)
+    return [address, 0, (<char *>&val[0])[:val.size()], bus]