-
Notifications
You must be signed in to change notification settings - Fork 7.7k
Remove problematic and unuseful logging statement from gpio.c (IDFGH-14846) #15569
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
Conversation
Removed inappropriate and misleading logging statement. The logging statement was at INFO level, which means that it was inappropriately logging cryptic info. And the context wasn't explained, which means that it took quite a long time to find out exactly why the logging statement wrongly implied that I wasn't successfully setting up input and output bits. Best to simply remove it.
👋 Hello skibu, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
Previous commit removed a logging statement. Turns out the variables used in the logging statement were not used for anything else. This meant that a compiler could complain about unused variables. Therefore simply removed the now unused variables.
I have now signed the contributor agreement. Do the commit titles need to be changed in order for this PR to be processed? I ask because this cannot be done via GitHub website. If the titles truly need to be changed then I have to download my fork and modify the commits manually. |
@skibu Don't think so, the log you removed is usefull for my debug, and can be bypassed by level. may I know how it misleads you, we can update it more understandable |
No offense, but seems like you're overreacting because you don't know what you're doing.
Non-sequitur
Care to elaborate on how that could be fixed?
How so? |
@wanckl , thank you for starting the dialog. I'm certain we can come up with a mutually satisfactory solution. What happened:
The reason this logging message was misleading is because it implied that after I setup my GPIO bit as an input or an output that InputEn:0 and OutputEn: 0. The message was implying that the bit was not actually successfully configured. The reason for this problem is that the logging message was part of a reset that was done before the bit was actually configured as an input or an output. But the message did not indicate that it was just part of the reset. Possible Solutions:
Let me know if I can further explain what happened. Also, let me know if you want to close this PR and have me submit a new one with a different solution. |
+1 for this option. I think the GPIO driver differs from other peripheral drivers in that it logs its configuration at info level. The reason why it differs is probably that GPIO driver was one of the first drivers we added in IDF, and there were no established conventions for logging from IDF components at that point. Calling |
Removed inappropriate and misleading logging statement. The logging statement was at INFO level, which means that it was inappropriately logging cryptic info. And the context wasn't explained, which means that it took quite a long time to find out exactly why the logging statement wrongly implied that I wasn't successfully setting up input and output bits. Best to simply remove it.