-
Notifications
You must be signed in to change notification settings - Fork 137
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
Fix permission denied error in linuxgpio programmer #917
Conversation
Thanks for the PR! I'm a pretty recent contributor, so @dl8dtl would have to have a look at this one. When searching for linuxgpio specific Avrdude issues it looks like there are related issues we well. You mentioned #386, but it looks to me like #472 and perhaps #372 reports the same issues too. |
This PR seems to help to avoid the issue.
|
But if the issue already happened (say using master without the fix), then it will not help.
In this case, PR #917 does not seem to be able to help any more.
|
All in all, I think this PR is good to be merged. It at least helps to avoid the issue in the first place. Moving to libgpiod may be the ultimate solution. |
Absolutely! Now that libgpiod is a part of the linux kernel, it looks like libgpiod is available under /linux/gpio.h, which is how But I have no idea how to port the existing linuxgpio code over to libgpiod. Maybe the person who write linuxgpio in the first place would be interested to look into it? |
@stefanrueger |
Not so sure who can review this and get this merged (or modified if necessary). My test results show it improves the situation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is competently coded, well done.
Small comments:
if (ret == 0) {
break;
} else if(...)
Whilst the code is not wrong, the else
is not warranted, as break
changes the flow of control. Below is arguably better readable:
if(ret==0)
break;
if(...)
Using printf() a no-no. It is better to use the avrdude_message()
system as it neatly considers the wanted verbosity and it prints to stderr, where all error and debug messages belong. I would change
if (retry_count > 1) {
printf("Retrying...\n");
}
to
if(retry_count > 1)
avrdude_message(MSG_NOTICE2, "%s: retrying linuxgpio_dir_%s()\n", progname, i == PIN_AVR_MISO? "in": "out");
or something to that effect (untested).
I would be happy to merge the PR as @mcuee found it a useful improvement. The one request I have is to replace the |
On Raspberry Pi using linuxgpio programmer without
sudo
fails withThe next attempt fails, because gpio pin remains exported and not removed.
Open issue: #386
Solution
After exporting gpio pin wait, until GPIO directory appears. Retry in case of EACCESS error while delayed udev permission rule applies. Call
unexport
, ifexport
failed for whatever reason.Verified that it fixes the problem.