woodstock Posted October 29, 2015 Report Share Posted October 29, 2015 Hi,I am very new to uGFX (discovered it two days ago!), but I have to say it is fantastic! When I compile my project with uGFX I get many warnings about "GFX_USE_OS_RAWRTOS" not being defined:ugfx/src/gos/gos_rules.h:27:140: warning: "GFX_USE_OS_RAWRTOS" is not defined [-Wundef]While this is not an error, it is slightly annoying. I have fixed this in my own working copy by adding the following lines into gos_options.h: /** * @brief Use RAWRTOS * @details Defaults to FALSE */ #ifndef GFX_USE_OS_RAWRTOS #define GFX_USE_OS_RAWRTOS FALSE #endifI have attached the patch for this. Does anyone else also get this warning?Using arm-none-wabi-gcc (version 4.9.3), Eclipse, ChibiOS 3.0, Mac OSX 10.11, compiling for STM32F103.I am also quite new to version management and working on open source projects, so I would like to ask what is the usual procedure for suggesting fixes like this? Should I submit a pull request?Woodstock[EDIT] It appears I cannot upload the attachment... Link to comment Share on other sites More sharing options...
Joel Bodenmann Posted October 29, 2015 Report Share Posted October 29, 2015 Hello woodstock and welcome to the community!Thank you for bringing this to our attention. We just fixed it: https://bitbucket.org/Tectu/ugfx/commit ... 3af9acbf6eWe appreciate your contribution.Regarding file uploads: I just double checked and file uploads are definitely allowed. However, as you mentioned that you uploaded the patch file we assume that it was a *.patch extension which was not whitelisted. We added this extension to the list of allowed extensions.For the future: You can get around that filter by just wrapping everything inside a ZIP archive Regarding contributions: What you did is certainly fine and the world would be a better place if everybody would be that thorough like you.The most important things are:Detailed explanation of what is wrong / what changed / what feature was added and a short test case that either allows to reproduce the problem or demonstrating the new featureAdding information about the used toolchain, platform, etc.Uploading a patch file to the forum postPull requests certainly work as well but personally we prefer to have a bare patch file attached to a forum post.~ Tectu Link to comment Share on other sites More sharing options...
inmarket Posted October 29, 2015 Report Share Posted October 29, 2015 It is interesting that you got warnings for this as it is part of the C preprocessor standard. A preprocessor variable not defined is treated as having a value of 0 (or false if tested as a boolean).Whilst we have added the define as we like to be complete with our definitions, you might want to tone down the compiler warning settings a little. Link to comment Share on other sites More sharing options...
woodstock Posted October 29, 2015 Author Report Share Posted October 29, 2015 Thank you for your help and for the commit.I should have noted that I have the following warning options enabled: -Wall -Wextra -Wundef -Wstrict-prototypes. Although I do like to be thorough with my warnings, maybe this is a little excessive...For small contributions (like this one), is it worth opening a new thread just for that single contribution?The reason I ask is because I have found another bug (I think ). See attached patch (I was not able to upload a *.patch file, so had to zip it as per your suggestion).patch.patch.zip Link to comment Share on other sites More sharing options...
Joel Bodenmann Posted October 29, 2015 Report Share Posted October 29, 2015 Thank you for your second patch. We will merge that ASAP.Generally we prefer one thread per incident to keep things clean. After all threads don't cost anything Thanks again for your contributions. They are very appreciated!~ Tectu Link to comment Share on other sites More sharing options...
Joel Bodenmann Posted October 30, 2015 Report Share Posted October 30, 2015 We just pushed your patch. Thank you once again!~ Tectu Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now