Add C code generation for value lists

[driftregion]

Hello @howerj , thanks for dbcc.

I too would like to add C code generation of value lists as enums. This was originally proposed in https://github.com/howerj/dbcc/pull/23. I made a minimal proof of concept:

https://github.com/driftregion/dbcc/commit/35f3beff63efe471393ccb2c1a7167d7c3bc67b2

vals_ast amiss

When starting out, I found that the vals_ast returned here is NULL: https://github.com/howerj/dbcc/blob/68139f75dd46742f3e1f03b38455f6caead93eb7/can.c#L334

example.dbc

VERSION ""


NS_ : 
	NS_DESC_
	CM_
	BA_DEF_
	BA_
	VAL_
	CAT_DEF_
	CAT_
	FILTER
	BA_DEF_DEF_
	EV_DATA_
	ENVVAR_DATA_
	SGTYPE_
	SGTYPE_VAL_
	BA_DEF_SGTYPE_
	BA_SGTYPE_
	SIG_TYPE_REF_
	VAL_TABLE_
	SIG_GROUP_
	SIG_VALTYPE_
	SIGTYPE_VALTYPE_
	BO_TX_BU_
	BA_DEF_REL_
	BA_REL_
	BA_DEF_DEF_REL_
	BU_SG_REL_
	BU_EV_REL_
	BU_BO_REL_
	SG_MUL_VAL_

BS_:

BU_: N1 N2

BO_ 690 N1_Event: 8 N1
 SG_ N1_Event M : 0|10@1+ (1,0) [0|0] ""  N2
 SG_ N1_Event000ecuReset m0 : 10|1@1+ (1,0) [0|0] "" N2
 SG_ N1_Event001bootFailureReason m1 : 10|4@1+ (1,0) [0|0] "" N2

CM_ SG_ 690 N1_Event001bootFailureReason "Why N1 failed to boot";
VAL_ 690 N1_Event 0 "N1_Event000ECURESET" 1 "N1_Event001FAILEDTOBOOT" ;
VAL_ 690 N1_Event001bootFailureReason 1 "BOOT_INFO_CRC_MISMATCH" 2 "APP_HEADER_CRC_MISMATCH" 3 "NODE_ENUM_MISMATCH" 4 "HW_VERSION_ENUM_MISMATCH" 5 "PROJECT_ENUM_MISMATCH" 6 "USAGE_ENUM_MISMATCH" 7 "APP_CRC_INVALID" ;

the AST shows that VAL_ is parsed as a types.

'
  types|> 
    ident|regex:46:1 'VAL_'
    s|regex:46:5 ' '
    whatever|integer|regex:46:6 '690'
    s|regex:46:9 ' '
    whatever|ident|regex:46:10 'N1_Event'
    s|regex:46:18 ' '
    whatever|integer|regex:46:19 '0'
    s|regex:46:20 ' '
    whatever|string|> 
      char:46:21 '"'
      regex:46:22 'N1_Event000ECURESET'
      char:46:41 '"'
    s|regex:46:42 ' '
    whatever|integer|regex:46:43 '1'
    s|regex:46:44 ' '
    whatever|string|> 
      char:46:45 '"'
      regex:46:46 'N1_Event001FAILEDTOBOOT'
      char:46:69 '"'
    s|regex:46:70 ' '
    char:46:71 ';'
    n|regex:46:72 '

what was once is no more

I found that behavior has changed since https://github.com/howerj/dbcc/pull/23 was merged (ce968d8132b4f7705450912bca1cebae79964421). On that commit, vals appears correctly in the AST.

'
  vals|> 
    val|> 
      string:44:1 'VAL_'
      s|regex:44:5 ' '
      id|integer|regex:44:6 '690'
      s|regex:44:9 ' '
      name|ident|regex:44:10 'N1_Event'
      s|regex:44:18 ' '
      val_item|> 
        integer|regex:44:19 '0'
        s|regex:44:20 ' '
        string|> 
          char:44:21 '"'
          regex:44:22 'N1_Event000ECURESET'
          char:44:41 '"'
        s|regex:44:42 ' '
      val_item|> 
        integer|regex:44:43 '1'
        s|regex:44:44 ' '
        string|> 
          char:44:45 '"'
          regex:44:46 'N1_Event001FAILEDTOBOOT'
          char:44:69 '"'
        s|regex:44:70 ' '
      char:44:71 ';'
      n|regex:44:72 '

the plucky git bisect run was given the following test

#! /usr/bin/env bash
make clean
make
./dbcc -vvvv -o tmp example.dbc > dbc.ast
grep vals dbc.ast

and found the key change in in https://github.com/howerj/dbcc/commit/94f604b45d5f5fc14731772bd43af4da98a7fd00#diff-a07721cd062be25900bddb926de15fc103cf32ea2726d1fea286f6548b810c6a

dbc       : <version> <symbols> <bs> <ecus> <values>* <n>* <messages> (<n>|<sigval>|<types>)*  ; \n" ;

I don't know the reason for removing <attribute_definition>, <attribute_value>* from the grammar. It seems related to the parsing of SIG_VALTYPE_. Some background here would be very helpful. When I added those tags back in, parsing worked as expected.

" dbc       : <version> <symbols> <bs> <ecus> <values>* <n>* <messages> <comments> <sigval>* <attribute_definition>* <attribute_value>* <vals>  ; \n" ;

why we want this

• to deduplicate DBC-defined value lists and user-application-defined enums (which may or may not exist)

design decisions

@howerj @martonmiklos , would appreciate your input

• how to handle duplicate enumerator identifiers ?

  • error, require that input DBC has no duplicate val_list item names
  • name mangle, prefix enumerator (val_list->name) with enum name (val_list_items[i]->name)
  • generate C++ namespaces (kidding)

• how to detect strings that are not valid C identifiers?

  • use the grammar (How?)

• how to handle strings that are not valid C identifiers?

  • warn, omit offending enum declaration, still generate code
  • warn, omit all enum declarations, still generate code
  • error, do not generate code

It might be worth generating functions for encoding/decoding and validating that specific enumeration, and then using those functions in the pack/print/unpack/encode/decode routines.

• pro: type checking, bounds checking
• con: type checking

[howerj]

Thanks for opening this issue, ideally they'd all be like this one, well documented and reasoned out.

I seem to recall the reason it was removed is that the grammar was causing problems with some DBC files for reasons I cannot remember and will have to remind myself of, and seeing as the program was not using the enum values, the offending bit in the grammar was removed. Development in this project is mainly driven by "why does this not work with my DBC file?".

To answer your questions:

I would probably handle duplicate enum values with your second option, by prefixing the enum with the enum name. It does generate long names, but I think it is sensible to do by default. A command line option could be given to surpress that behaviour, but I would only add that if someone actually asked for it.

You could use the grammar to detect valid C identifiers, the "ident" type encodes most of the rules that detemines whether an id is valid (as both a C id and a DBC one). By adding the suffix "_e" to the generated enumeration values as well then you could avoid having to detect whether or not an enum if a keyword as well.

Looking at the commit you've made, there's still more to do, but it's a good start, and it does not seem to break anything. It might be worth developing a branch "feature/enum" before merging into master. I'd like to add bounds checking and printing before merging.