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

Flush function can't be omitted #197

Closed
rikvdh opened this issue May 30, 2016 · 16 comments
Closed

Flush function can't be omitted #197

rikvdh opened this issue May 30, 2016 · 16 comments

Comments

@rikvdh
Copy link
Contributor

rikvdh commented May 30, 2016

Hi,

We are using Unity with output-char putting a character directly to the console. But we can't omit the new Flush function added in commit c5c392b.

When I define UNITY_OUTPUT_FLUSH or UNITY_OUTPUT_FLUSH() it fails at the https://github.com/ThrowTheSwitch/Unity/blob/master/src/unity_internals.h#L308 declaration.

For -D'UNITY_OUTPUT_FLUSH()':
error: macro "UNITY_OUTPUT_FLUSH" passed 1 arguments, but takes just 0

for -DUNITY_OUTPUT_FLUSH:

error: expected identifier or '(' before numeric constant
../Unity/src/unity_internals.h:308:13: note: in expansion of macro 'UNITY_OUTPUT_FLUSH'
 extern void UNITY_OUTPUT_FLUSH(void);
             ^

for -DUNITY_OMIT_OUTPUT_FLUSH_HEADER_DECLARATION -DUNITY_OUTPUT_FLUSH:

<command-line>:0:20: error: expected identifier or '(' before numeric constant
../Unity/src/unity.c:15:5: note: in expansion of macro 'UNITY_OUTPUT_FLUSH'
 int UNITY_OUTPUT_FLUSH(void);
     ^

for -DUNITY_OMIT_OUTPUT_FLUSH_HEADER_DECLARATION -D'UNITY_OUTPUT_FLUSH()':

../Unity/src/unity.c:15:28: error: macro "UNITY_OUTPUT_FLUSH" passed 1 arguments, but takes just 0
 int UNITY_OUTPUT_FLUSH(void);
                            ^

I'm out of combinations. I also tried thinks like: -D'UNITY_OUTPUT_FLUSH()'='(void)'
I just want to omit this feature which is unneeded for our case.

Having odd forward declarations all over the shop seems a bit odd. Perhaps a UNITY_HAS_FLUSH and UNITY_FLUSH_FORWARD_DECL config options are more desirable than the current situation.

Kind regards,

Rik

@mvandervoord
Copy link
Member

Rik:

Thanks for delving into this issue! I've reproduced your results and agree with all your findings. I'm also not happy with how complex this minor feature is at the moment... it's something that is rarely used but is getting complicated for people to just ignore.

The biggest problem with the current implementation is that it really wants to detect if the UNITY_OUTPUT_FLUSH macro has been defined as nothing. The only good way to do that is to use VA_ARGS which isn't standard. So that's out.

I'll fiddle around with it a bit and get a something working. It might be something along the lines of what you suggest. If anyone else has thoughts, now is the time. ;)

@xor-gate
Copy link
Contributor

xor-gate commented May 30, 2016

Current code is way to complex in configuration and you get a #define dependency hell which will kills users in-battle who have old or not-often-used-set-of-defines. Like this:

goto

Also the configuration template should not be placed under examples/unity_config.h. At least it should be mentioned in the README it exists....

@mvandervoord
Copy link
Member

snicker Well that was a colorful statement of support for revising this feature!

@xor-gate
Copy link
Contributor

I'm just joking around, I highly appreciate all contributions and hard work on the Unity library 👍. We should watch out we will not be delving into to much configuration options and should work-out-of-the-box(tm).

@mvandervoord
Copy link
Member

:) Anytime you can use XKCD in a comment is worthwhile.

Anyway, your statement on working out-of-the-box is exactly our goal. We try to make it Just Work (TM) for most users and then allow them to override default behavior where needed. This feature is definitely failing to meet that criteria at this point!

@xor-gate
Copy link
Contributor

xor-gate commented May 30, 2016

Good that we look all at the right direction!

@rikvdh
Copy link
Contributor Author

rikvdh commented May 30, 2016

perhaps you can implicitly define UNITY_OUTPUT_FLUSH to fflush when you have #ifdef __linux__ and omit int UNITY_OUTPUT_FLUSH(void) when #if !defined(UNITY_OUTPUT_FLUSH) && !defined(__linux__).

@mvandervoord
Copy link
Member

I've made a few more tweaks to this feature. On my machine I can now do any of these without getting any errors or warnings. Travis also seems happy.

  • define UNITY_OUTPUT_FLUSH to nothing.
  • define UNITY_OUTPUT_FLUSH to be a custom macro
  • leave UNITY_OUTPUT_FLUSH undefined.

Since two machines aren't enough evidence to call something fixed, would you guys mind checking your own setups to see how the MAIN branch works for you?

@xor-gate
Copy link
Contributor

xor-gate commented Jun 21, 2016

@mvandervoord thank you for your work, we will probably have some time to test next week.

@mvandervoord
Copy link
Member

Cool. Let me know. :)

@xor-gate
Copy link
Contributor

@mvandervoord I have tested it and it works (for the flush function).
We only specifiy our own UNITY_OUTPUT_CHAR and seems to be enough.

@rikvdh you may close this issue.

@mvandervoord
Copy link
Member

awesome. Thanks for checking, @xor-gate

@xor-gate
Copy link
Contributor

I only got caught with a different problem but this has nothing todo with the flush see issue #200.

@xor-gate
Copy link
Contributor

@rikvdh was way to busy to react :+). Thanks for closing @mvandervoord

@mvandervoord
Copy link
Member

:) I figure if he disagrees, he's welcome to reopen it.

@xor-gate
Copy link
Contributor

xor-gate commented Jun 29, 2016

He sits in front of my desk, so I will hear it way earlier than the issue is reopened :+). Thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants