MITMotorsports / CANlib

An automatically generated C library for packing and unpacking based on ParseCAN specifications.
Apache License 2.0
5 stars 3 forks source link

Major refactor #3

Closed nistath closed 6 years ago

nistath commented 6 years ago

This is a major refactor of the CANlib and the generator, with a few goals:

This refactor is a middle path -- not yet eradicating drivers.

Changes

It's helpful to read common.py first.

Buses, messages, and segments

constants.h includes definitions for message attributes, such as can_id and frequency. Those are called by coord(bus.name, msg.name, attrnm)

Can_Library.h includes definitions for message forms, enumerating the different forms that can be received. There can be a one to many mapping between a CAN message identified by its can_id and a message form -- allowing for concepts such as multiplexing. Those are called by coord(bus.name, msg.name).

All message forms have their own struct, defined in structs.c. Those structs are called by '{}_T'.format(coord(bus.name, msg.name)).

Segments still get their own enums and are their own types in enum_segments.c, just with better naming. Those types are called by '{}_T'.format(coord(bus.name, msg.name, seg.name)) and the included enum values by coord(bus.name, msg.name, seg.name, val.name). It would be a problem is someone had a value named T -- this nuisance exists in many other places -- we'll avoid it manually via spec for now.

Generator

In common.py, the coord function is an attempt to standardize naming. Ideally there would exist functions that take a bus[->message[->segment]] coordinate and return the proper name. This would avoid confusion between multiple files needing to name things based on the same concept.

nistath commented 6 years ago

Do you think that refactoring this further could be a freshman project next year? :P

nistath commented 6 years ago

@sebLopezCot, there is a potential naming bug lying in bus.name_anything so I chose caps just to reduce the probability of it happening. The conflict would exist if you had a message named IDENTIFY in any bus (conflicts with message form names formatted bus.name_msg.name).

How about renaming to identify_bus.name and init_bus.name_board.name?

nistath commented 6 years ago

Also, if we have _ as the official delimiter, we might as well not have _ in the spec names to avoid confusion. Unless there's an alternative.

sebLopezCot commented 6 years ago

Things look good on stm32f2, should probs test on an lpc before approving...

nistath commented 6 years ago

LPC works with 2 CAN nodes. Minor changes coming before merge. @sebLopezCot

sebLopezCot commented 6 years ago

New changes look good and compile on the stm32f2, didn't test, but changes are minor. Don't know if the last commit is tested on LPC, but I think we're fine here. Nice work team!!!