Skip to content

msp430: add assert header#2440

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64:msp430/api/add-assert
Feb 12, 2015
Merged

msp430: add assert header#2440
miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64:msp430/api/add-assert

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Feb 11, 2015

The assert() macro is needed for utlist (which is used in #2285 and #2404). Therefore we need an implementation of assert().

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer labels Feb 11, 2015
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.

Reading the docs, you already implemented behaviour for NDEBUG defined?

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Feb 12, 2015

Addressed comments

@thomaseichinger
Copy link
Copy Markdown
Member

ACK when squashed.

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.

Why not implement the rest right now?

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.

Because I was not sure if abort() is implemented on msp430

@miri64 miri64 force-pushed the msp430/api/add-assert branch from 3bb8ab1 to 722a9e7 Compare February 12, 2015 11:49
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Feb 12, 2015

Added implementation. Decided to use DEBUGF instead of printf, to save the stack memory (also so I do not need to implement the formatting ^^), in case it is called. If one wants to enable output they can always use ENABLE_DEBUG.

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.

done?

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.

Yes. Removed this line

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Feb 12, 2015

Addressed comments

@haukepetersen
Copy link
Copy Markdown
Contributor

ACK when squashed.

@miri64 miri64 force-pushed the msp430/api/add-assert branch from d0ed730 to f00e532 Compare February 12, 2015 13:08
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Feb 12, 2015

Squashed. Waiting for travis

@miri64 miri64 force-pushed the msp430/api/add-assert branch from f00e532 to 9077299 Compare February 12, 2015 13:24
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Feb 12, 2015

Addressed @LudwigOrtmann's post-ACK comment. Can we merge it now…

@LudwigKnuepfer
Copy link
Copy Markdown
Member

ACK ACK

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Feb 12, 2015

Waiting for travis

miri64 added a commit that referenced this pull request Feb 12, 2015
@miri64 miri64 merged commit f3fccf2 into RIOT-OS:master Feb 12, 2015
@miri64 miri64 deleted the msp430/api/add-assert branch February 12, 2015 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants