Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

msp430: add assert header #2440

Merged
merged 1 commit into from
Feb 12, 2015
Merged

Conversation

miri64
Copy link
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
/**
* @see http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/assert.h.html
*
* @todo implement behaviour for NDEBUG defined.
Copy link
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
Member Author

miri64 commented Feb 12, 2015

Addressed comments

@thomaseichinger
Copy link
Member

ACK when squashed.

/**
* @see http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/assert.h.html
*
* @todo implement behaviour for NDEBUG not defined.
Copy link
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
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
Copy link
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.

* @def assert
* @see http://pubs.opengroup.org/onlinepubs/9699919799/functions/assert.html
*
* @todo implement behaviour for NDEBUG not defined.
Copy link
Member

Choose a reason for hiding this comment

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

done?

Copy link
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
Member Author

miri64 commented Feb 12, 2015

Addressed comments

@haukepetersen
Copy link
Contributor

ACK when squashed.

@miri64
Copy link
Member Author

miri64 commented Feb 12, 2015

Squashed. Waiting for travis

@miri64
Copy link
Member Author

miri64 commented Feb 12, 2015

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

@LudwigKnuepfer
Copy link
Member

ACK ACK

@miri64
Copy link
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