Skip to content

can: add tja1042 transceiver driver#6925

Merged
vincent-d merged 2 commits intoRIOT-OS:masterfrom
OTAkeys:pr/tja1042
Jan 18, 2018
Merged

can: add tja1042 transceiver driver#6925
vincent-d merged 2 commits intoRIOT-OS:masterfrom
OTAkeys:pr/tja1042

Conversation

@vincent-d
Copy link
Copy Markdown
Member

Follow up, based on #5793

This is the TJA1042 (http://www.nxp.com/documents/data_sheet/TJA1042.pdf) HS CAN transceiver driver.

@miri64 miri64 requested a review from aabadie May 3, 2017 12:09
@miri64 miri64 added the Area: drivers Area: Device drivers label May 3, 2017
@aabadie aabadie added this to the Release 2017.07 milestone Jun 21, 2017
@vincent-d vincent-d force-pushed the pr/tja1042 branch 3 times, most recently from 42598a7 to 2f6dddf Compare June 26, 2017 15:14
@vincent-d
Copy link
Copy Markdown
Member Author

vincent-d commented Jun 26, 2017

Rebased and updated, added an extra test app. Still need to re-test it.

@smlng smlng added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 15, 2018
Copy link
Copy Markdown
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mainly doxygen related, hence update docu to common usage in RIOT (see other drivers for grouping, for instance).

*/

/**
* @defgroup tja1042 TJA1042
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be drivers_tja1042


/**
* @defgroup tja1042 TJA1042
* @ingroup trx_can
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiple groups are possible, add @ingroup drivers here, too

#endif

/**
* tja1042 can trx descriptor
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing @brief

include ../Makefile.tests_common


CFLAGS += -DDEVELHELP
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be removed, set in tests_common

@vincent-d
Copy link
Copy Markdown
Member Author

@smlng comments addressed

Copy link
Copy Markdown
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nits

*/

/**
* @ingroup tja1042 trx
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be @ingroup drivers_tja1042 only

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

though also these files are not parsed by doxygen, anyway.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed

@smlng
Copy link
Copy Markdown
Member

smlng commented Jan 17, 2018

looks good, please squash.

@vincent-d
Copy link
Copy Markdown
Member Author

All green, go!

@vincent-d vincent-d merged commit e6ca1af into RIOT-OS:master Jan 18, 2018
@vincent-d vincent-d deleted the pr/tja1042 branch January 18, 2018 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants