search

JamesBarwell / rpi-gpio.js

Control Raspberry Pi GPIO pins with node.js
MIT License
658 stars 115 forks source link

address pin config race #49

Closed aztecrex closed 7 years ago

aztecrex commented 7 years ago

The edge and direction are set after a pin is exported. Because ownership is set to gpio asynchronously after export, immediately setting the edge and setting the direction can fail if the user is not root.

This patch retries each operation up to 100 times at .01s intervals, resulting in a successful pin setup for a non-root user.

awong-dev commented 7 years ago

FYI, this matches the workaround from the python RPI-GPIO package. See line 106 here: https://sourceforge.net/p/raspberry-gpio-python/code/ci/default/tree/source/event_gpio.c

Snippet:

    // retry waiting for udev to set correct file permissions
    delay.tv_sec = 0;
    delay.tv_nsec = 10000000L; // 10ms
    for (retry=0; retry<100; retry++) {
        if ((fd = open(filename, O_WRONLY)) >= 0)
            break;
        nanosleep(&delay, NULL);
    }
    if (retry >= 100)
        return -1;
awong-dev commented 7 years ago

Oh, and I think your PR comment also needs an update since it's now 10 times at .001sec.

Also...thanks for writing this PR! I'm a node.js noob so I had no idea how to do this right :)

JamesBarwell commented 7 years ago

Hi, thanks for the patch and for all the investigating into the issue.

I'm keen to merge but also keen that every feature merged has the relevant test coverage to prevent a future regression. Perhaps just something as simple as making setEdge fail on the first call, then asserting that it is called again will do. Any chance you could add this to the PR? Cheers.

aztecrex commented 7 years ago

No problem. Will be a few days before I can get to it.

aztecrex commented 7 years ago

Just trying to run npm test and I get this. Any idea what's going on? Maybe a global dependency?

aztecrex@trefoil:~/Code/rpi-gpio.js[]$ npm test

> rpi-gpio@0.7.0 test /Users/aztecrex/Code/rpi-gpio.js
> mocha

/Users/aztecrex/Code/rpi-gpio.js/node_modules/bindings/bindings.js:83
        throw e
        ^

Error: Module did not self-register.
    at Object.Module._extensions..node (module.js:598:18)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)
    at bindings (/Users/aztecrex/Code/rpi-gpio.js/node_modules/bindings/bindings.js:76:44)
    at Object.<anonymous> (/Users/aztecrex/Code/rpi-gpio.js/node_modules/epoll/epoll.js:1:99)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/Users/aztecrex/Code/rpi-gpio.js/test/main.js:13:1)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)
    at /Users/aztecrex/Code/rpi-gpio.js/node_modules/mocha/lib/mocha.js:222:27
    at Array.forEach (native)
    at Mocha.loadFiles (/Users/aztecrex/Code/rpi-gpio.js/node_modules/mocha/lib/mocha.js:219:14)
    at Mocha.run (/Users/aztecrex/Code/rpi-gpio.js/node_modules/mocha/lib/mocha.js:487:10)
    at Object.<anonymous> (/Users/aztecrex/Code/rpi-gpio.js/node_modules/mocha/bin/_mocha:459:18)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.runMain (module.js:605:10)
    at run (bootstrap_node.js:420:7)
    at startup (bootstrap_node.js:139:9)
    at bootstrap_node.js:535:3
npm ERR! Test failed.  See above for more details.
JamesBarwell commented 7 years ago

I can see someone with the same issue here: http://stackoverflow.com/questions/38040653/bindings-module-did-not-self-register

It could be your C compiler. If you look in the travis file here you can see how it is set up to run on Ubuntu: https://github.com/JamesBarwell/rpi-gpio.js/blob/master/.travis.yml

If you can't get it working, I'd suggest just doing your best and pushing patches up to github. Travis should automatically run the tests and you'll see here if it passes. Doesn't matter if there are a few dodgy commits in the pull request from trial and error - we can always rebase it before merge!

JamesBarwell commented 7 years ago

I had the same issue with the tests just now, and had to run apt-get install g++ to get the missing dependency that Epoll required.

Anyway, I've been testing your patch this morning, and it has been working nicely for me on a Model B with node 6.2.1. Although the test coverage is missing, I was able to use the existing integration tests to prove that it works.

I've had a go myself at implementing the test and it's not that simple due to the way the filesystem all gets stubbed out at the top of the tests. It's a bit of a pain in fact and I think I've got a job to refactor the tests a bit. So my plan now is to just get this merged in and get it out there, using the integration coverage to ensure no regressions, and I'll look to implement a proper test myself in the future.