:source-highlighter: highlight.js = The issues with NMEA2000 == The current code is broken The (single) regression file has packets from four different devices. The Garmin AIS600 and GPS17x do not generate consistent results. The depth sounder and the battery monitor(?) do not yield results. The code splits up the incoming data stream among several gps_device_t entries; we do not reassign them to an appropriate device. We write packets that can be parsed individually to the out buffer, overwriting any data already there; packets part of a sequence likewise overwrite the in buffer. == We should not require real hardware for development The in-tree code uses socketCAN, a Linux-specific extension that removes the need to deal with the hardware directly. The current and all known proposed code ignores CAN bus address wrangling. To keep things dumber, I make some assumptions. - Devices always retain the same address in a session - Fast sequences always ship packets in ascending order - Sequences from a device complete before starting a new sequence - GPSD will ship TPV components (precisely depth) without a time set - I will remember what the rest of the list was == My proposed changes to the find_pgn function I recommend diminishing the arrow anti-pattern by means like the following. It's nice to limit the arrow anti-pattern to preserve a comfy 72-column maximum. repeating the following several times can reduce the width of the codebase. .before [source,python] ---- if something: long_complicated_logic() else: pass ---- .after [source,python] ---- if not something: return long_complicated_logig() ---- I made an exception for the new device handling code, as returning there would effectively drop a packet. We should enable the fast sequence logging at a lower tier and revise it for clarity. I should comment on the code more, and there should be a page in www/ documenting some of this. I understand the benefit of adding a few new ingress modes. One is a straight import of the candump format; abridging yields another format with an ampersand rather than the timestamp, bus, and spaces. Another couple of possible ingress modes are from a serial dongle or CAN via UDP; neither is a priority at the moment, the former from perceived difficulty, and both from lack of demand. -- (1410714882.622188) can0 09F80102#36729A16021AB80D &09F80202#D0FC00000300FFFF -- == Snakey pseudocode for existing find_pgn() [source,python] ---- def find_pgn(frame, session): session.driver.nmea2000.workpgn = None can_net = session.driver.nmea2000.can_net if (NMEA2000_NETS-1) < can_net: log.error("NMEA2000 find_pgn: Invalid can network %d.\n", can_net) return if 0 != (CAN_EFF_FLAG & frame.can_id): session.driver.nmea2000.can_msgcnt += 1 source_pgn = (frame.can_id >> 8) & 0x1ffff source_prio = (frame.can_id >> 26) & 0x7 source_unit = frame.can_id & 0x0ff daddr = 0xff if 240 > ((source_pgn >> 8) & 0xff): daddr = source_pgn & 0x00ff source_pgn = source_pgn & 0x1ff00 log.data("nmea2000: source_prio %u daddr %u", source_pgn, daddr) if not session.driver.nmea2000.unit_valid: for l1 in range(NMEA2000_NETS): for l2 in range(NMEA2000_UNITS): if session == nmea2000_units[l1][l2]: session.driver.nmea2000.unit = l2 session.driver.nmea2000.unit_valid = True session.driver.nmea2000.can_net = l1 can_net = l1 session.driver.nmea2000.unit = source_unit session.driver.nmea2000.unit_valid = true nmea2000_units[can_net][source_unit] = session if source_unit == session.driver.nmea2000.unit: if session.driver.nmea2000.pgnlist: work = search_pgnlist(source_pgn, session.driver.nmea2000.pgnlist) else: pgnlist = gpspgn work = search_pgnlist(source_pgn, pgnlist) if not work: pgnlist = aispgn work = search_pgnlist(source_pgn, pgnlist) if not work: pgnlist = pwrpgn work = search_pgnlist(source_pgn, pgnlist) if not work: pgnlist = navpgn work = search_pgnlist(source_pgn, pgnlist) if work is None and 0 < work.type: session.driver.nmea2000.pgnlist = pgnlist if work is not None: if 0 == work.fast: log_data("pgn %6d:%s \n", work.pgn, work.name) session.driver.nmea2000.workpgn = work session.lexer.outbuflen = frame.can_dlc & 0x0f for l2 in range(session.lexer.outbuflen): session.lexer.outbuffer[l2]= frame.data[l2] elif 0 == (frame.data[0] & 0x1f): session.driver.nmea2000.fast_packet_len = frame.data[1] session.driver.nmea2000.idx = frame.data[0] session.lexer.inbuflen = 0 session.driver.nmea2000.idx += 1 for l2 in range(2, 8): session.lexer.inbuffer[session.lexer.inbuflen++] = frame.data[l2] log.data("pgn %6d:%s \n", work.pgn, work.name) elif (frame.data[0] == session.driver.nmea2000.idx): for l2 in range(2, 8): if session.driver.nmea2000.fast_packet_len > session.lexer.inbuflen: session.lexer.inbuffer[session.lexer.inbuflen++] = frame.data[l2] if session.lexer.inbuflen == session.driver.nmea2000.fast_packet_len: session.driver.nmea2000.workpgn = work session.lexer.outbuflen = session.driver.nmea2000.fast_packet_len for l2 in range(session.lexer.outbuflen): session.lexer.outbuffer[l2] = session.lexer.inbuffer[l2] session.driver.nmea2000.fast_packet_len = 0 else: session.driver.nmea2000.idx += 1 else: log.error("Fast error %2x %2x %2x %2x %6d\n", session.driver.nmea2000.idx, frame.data[0], session.driver.nmea2000.unit, (unsigned int)session.driver.nmea2000.fast_packet_len, source_pgn) else: log.warn("PGN not found %08d %08x \n", source_pgn, source_pgn) else: # we got a unknown unit number if (nmea2000_units[can_net][source_unit] == NULL): buffer = "nmea2000:#%s:%u" % ( can_interface_name[can_net], source_unit) if gpsd_add_device: gpsd_add_device(buffer, true) else: pass ---- == Snakey pseudocode for propose find_pgn() replacement [source,python] ---- def find_pgn(frame, session): session.driver.nmea2000.workpgn = None can_net = session.driver.nmea2000.can_net if (NMEA2000_NETS-1) < can_net: log.error("NMEA2000 find_pgn: Invalid can network %d.\n", can_net) return if 0 == (CAN_EFF_FLAG & frame.can_id): return session.driver.nmea2000.can_msgcnt += 1 source_pgn = (frame.can_id >> 8) & 0x1ffff source_prio = (frame.can_id >> 26) & 0x7 source_unit = frame.can_id & 0x0ff daddr = 0xff if 240 > ((source_pgn >> 8) & 0xff): daddr = source_pgn & 0x00ff source_pgn = source_pgn & 0x1ff00 log.data("nmea2000: source_prio %u daddr %u", source_pgn, daddr) if not session.driver.nmea2000.unit_valid: for l1 in range(NMEA2000_NETS): for l2 in range(NMEA2000_UNITS): if session == nmea2000_units[l1][l2]: session.driver.nmea2000.unit = l2 session.driver.nmea2000.unit_valid = True session.driver.nmea2000.can_net = l1 can_net = l1 session.driver.nmea2000.unit = source_unit session.driver.nmea2000.unit_valid = true nmea2000_units[can_net][source_unit] = session if source_unit != session.driver.nmea2000.unit: # we got a unknown unit number if (nmea2000_units[can_net][source_unit] == NULL): buffer = "nmea2000:#%s:%u" % ( can_interface_name[can_net], source_unit) if gpsd_add_device: gpsd_add_device(buffer, true) if session.driver.nmea2000.pgnlist: work = search_pgnlist(source_pgn, session.driver.nmea2000.pgnlist) else: pgnlist = gpspgn work = search_pgnlist(source_pgn, pgnlist) if not work: pgnlist = aispgn work = search_pgnlist(source_pgn, pgnlist) if not work: pgnlist = pwrpgn work = search_pgnlist(source_pgn, pgnlist) if not work: pgnlist = navpgn work = search_pgnlist(source_pgn, pgnlist) if work is None and 0 < work.type: session.driver.nmea2000.pgnlist = pgnlist if work is None: log.warn("PGN not found %08d %08x \n", source_pgn, source_pgn) return if 0 == work.fast: log_data("pgn %6d:%s \n", work.pgn, work.name) session.driver.nmea2000.workpgn = work session.lexer.outbuflen = frame.can_dlc & 0x0f log.data("%s:%d short PGN%06d ]%s[", can_interface_name[can_net], source_unit, source_pgn, work->name) work.func(frame.data, session) elif 0 == (frame.data[0] % 32): session.driver.nmea2000.fast_packet_len = frame.data[1] session.driver.nmea2000.idx = frame.data[0] session.lexer.inbuflen = 0 session.driver.nmea2000.idx += 1 session.lexer.outbuffer[-256:-249] = frame.data[1:] log.data( 'Start fast assembly %s:%d PGN%06d ]%s[ want %d of seq %02d\n', can_interface_name[can_net], source_unit, source_pgn, work->name, frame.data[1], frame.data[0] // 32 ) elif (frame.data[0] == session.driver.nmea2000.idx): _off = 7 * (frame.data[0] % 32) session.lexer.outbuffer[off - 256:off - 249] = frame.data[1:] if session.lexer.inbuflen == session.driver.nmea2000.fast_packet_len: log.data( 'finish fast assembly of %s:%d PGN%06d %d/%d of seq %02d\n', can_interface_name[can_net], source_unit, source_pgn, (7 * session.driver.nmea2000.idx) - 1, session.driver.nmea2000.fast_packet_len, frame.data[0] // 32 ) session.driver.nmea2000.workpgn = work work.func(session.lexer.outbuffer[-256:], session) session.driver.nmea2000.fast_packet_len = 0 else: log.data( 'continue fast assembly of %s:%d PGN%06d %d/%d of seq %02d\n', can_interface_name[can_net], source_unit, source_pgn, (7 * session.driver.nmea2000.idx) - 1, session.driver.nmea2000.fast_packet_len, frame.data[0] // 32 ) session.driver.nmea2000.idx += 1 else: log.error("Fast error %2x %2x %2x %2x %6d\n", session.driver.nmea2000.idx, frame.data[0], session.driver.nmea2000.unit, (unsigned int)session.driver.nmea2000.fast_packet_len, source_pgn) ---- //end