From 5ef27b48664f5fcb7a40cf6f4a5a8d61a2ee44b0 Mon Sep 17 00:00:00 2001 From: Dean Lee Date: Wed, 5 Jun 2024 02:31:42 +0800 Subject: [PATCH 1/3] improve logging (#1048) --- .pre-commit-config.yaml | 1 + can/common.h | 1 + can/logger.h | 27 +++++++++++++++++++++++++++ can/parser.cc | 7 +++---- 4 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 can/logger.h diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7c45c439e2..8b541af667 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -48,6 +48,7 @@ repos: args: - --error-exitcode=1 - --language=c++ + - --inline-suppr - --force - --quiet - -j4 diff --git a/can/common.h b/can/common.h index cfc63eb105..80897966f8 100644 --- a/can/common.h +++ b/can/common.h @@ -13,6 +13,7 @@ #include "cereal/gen/cpp/log.capnp.h" #endif +#include "opendbc/can/logger.h" #include "opendbc/can/common_dbc.h" #define INFO printf diff --git a/can/logger.h b/can/logger.h new file mode 100644 index 0000000000..bab70c4894 --- /dev/null +++ b/can/logger.h @@ -0,0 +1,27 @@ +#pragma once + +#ifdef SWAGLOG +// cppcheck-suppress preprocessorErrorDirective +#include SWAGLOG +#else + +#define CLOUDLOG_DEBUG 10 +#define CLOUDLOG_INFO 20 +#define CLOUDLOG_WARNING 30 +#define CLOUDLOG_ERROR 40 +#define CLOUDLOG_CRITICAL 50 + +#define cloudlog(lvl, fmt, ...) printf(fmt "\n", ## __VA_ARGS__) +#define cloudlog_rl(burst, millis, lvl, fmt, ...) printf(fmt "\n", ##__VA_ARGS__) + +#define LOGD(fmt, ...) cloudlog(CLOUDLOG_DEBUG, fmt, ## __VA_ARGS__) +#define LOG(fmt, ...) cloudlog(CLOUDLOG_INFO, fmt, ## __VA_ARGS__) +#define LOGW(fmt, ...) cloudlog(CLOUDLOG_WARNING, fmt, ## __VA_ARGS__) +#define LOGE(fmt, ...) cloudlog(CLOUDLOG_ERROR, fmt, ## __VA_ARGS__) + +#define LOGD_100(fmt, ...) cloudlog_rl(2, 100, CLOUDLOG_DEBUG, fmt, ## __VA_ARGS__) +#define LOG_100(fmt, ...) cloudlog_rl(2, 100, CLOUDLOG_INFO, fmt, ## __VA_ARGS__) +#define LOGW_100(fmt, ...) cloudlog_rl(2, 100, CLOUDLOG_WARNING, fmt, ## __VA_ARGS__) +#define LOGE_100(fmt, ...) cloudlog_rl(2, 100, CLOUDLOG_ERROR, fmt, ## __VA_ARGS__) + +#endif diff --git a/can/parser.cc b/can/parser.cc index 3ed9f02880..3efa3ee369 100644 --- a/can/parser.cc +++ b/can/parser.cc @@ -10,7 +10,6 @@ #include #include -#include "cereal/logger/logger.h" #include "opendbc/can/common.h" int64_t get_raw_value(const std::vector &msg, const Signal &sig) { @@ -65,7 +64,7 @@ bool MessageState::parse(uint64_t nanos, const std::vector &dat) { // only update values if both checksum and counter are valid if (checksum_failed || counter_failed) { - LOGE("0x%X message checks failed, checksum failed %d, counter failed %d", address, checksum_failed, counter_failed); + LOGE_100("0x%X message checks failed, checksum failed %d, counter failed %d", address, checksum_failed, counter_failed); return false; } @@ -290,9 +289,9 @@ void CANParser::UpdateValid(uint64_t nanos) { if (state.check_threshold > 0 && (missing || timed_out)) { if (show_missing && !bus_timeout) { if (missing) { - LOGE("0x%X '%s' NOT SEEN", state.address, state.name.c_str()); + LOGE_100("0x%X '%s' NOT SEEN", state.address, state.name.c_str()); } else if (timed_out) { - LOGE("0x%X '%s' TIMED OUT", state.address, state.name.c_str()); + LOGE_100("0x%X '%s' TIMED OUT", state.address, state.name.c_str()); } } _valid = false; From dfaa27d31344fcf66a9656dffbaf5469c0674f0a Mon Sep 17 00:00:00 2001 From: Dean Lee Date: Wed, 5 Jun 2024 03:02:23 +0800 Subject: [PATCH 2/3] CANPacker: fix incorrect `counters` assignment (#1047) * fix counter * catch this issue in test case --- can/packer.cc | 4 ++-- can/tests/test_packer_parser.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/can/packer.cc b/can/packer.cc index 87fa0a8201..66b6a49483 100644 --- a/can/packer.cc +++ b/can/packer.cc @@ -61,9 +61,9 @@ std::vector CANPacker::pack(uint32_t address, const std::vector Date: Wed, 5 Jun 2024 23:08:15 -0700 Subject: [PATCH 3/3] assert expected openpilot undefined message behavior (#1050) assert expected openpilot behavior --- can/tests/test_packer_parser.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/can/tests/test_packer_parser.py b/can/tests/test_packer_parser.py index 79f24ec5aa..c313805c47 100755 --- a/can/tests/test_packer_parser.py +++ b/can/tests/test_packer_parser.py @@ -382,6 +382,18 @@ def test_disallow_duplicate_messages(self): with self.assertRaises(RuntimeError): CANParser("toyota_nodsu_pt_generated", [("ACC_CONTROL", 10), ("ACC_CONTROL", 10)]) + def test_allow_undefined_msgs(self): + # TODO: we should throw an exception for these, but we need good + # discovery tests in openpilot first + packer = CANPacker("toyota_nodsu_pt_generated") + + self.assertEqual(packer.make_can_msg("ACC_CONTROL", 0, {"UNKNOWN_SIGNAL": 0}), + [835, 0, b'\x00\x00\x00\x00\x00\x00\x00N', 0]) + self.assertEqual(packer.make_can_msg("UNKNOWN_MESSAGE", 0, {"UNKNOWN_SIGNAL": 0}), + [0, 0, b'', 0]) + self.assertEqual(packer.make_can_msg(0, 0, {"UNKNOWN_SIGNAL": 0}), + [0, 0, b'', 0]) + if __name__ == "__main__": unittest.main()