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

add rcServo,rcIMU,rcBMP functions #178

Merged
merged 2 commits into from
Jul 27, 2018
Merged

Conversation

vaishnavachath
Copy link
Contributor

changes made to node-roboticscape: jadonk/node-roboticscape#5

tested on:
BeagleBone Blue BeagleBoard.org Debian Image 2018-03-05
Linux beaglebone 4.9.105-ti-r113
rc_version : 0.4.4

changes made to node-roboticscape: jadonk/node-roboticscape#5

tested on:
BeagleBone Blue BeagleBoard.org Debian Image 2018-03-05
Linux beaglebone 4.9.105-ti-r113
rc_version : 0.4.4
Copy link
Owner

@jadonk jadonk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update package.json so the library moves to 0.0.8 as an optional dependency and handle the case where the library is old or the module doesn't build. What happens if you try to run on an old image with a 0.4.1 library installed? Can we fail gracefully?

@@ -18,7 +18,7 @@ var autorun = require('./autorun');
var server = require('./server');
var socketHandlers = require('./socket_handlers');
var ffi = require('./ffiimp');
//var rc = require('./rc');
var rc = require('./rc');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requires an package.json update to 0.0.8.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, can we use the optional require here in case the package doesn't install nice?

/*for(var x in rc) {
exports[x] = rc[x];
}*/
for (var x in rc) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you handle the condition that the library isn't installed nice?

@vaishnavachath
Copy link
Contributor Author

@jadonk i have updated the version in package.json ,"roboticscape" is already listed as an optional dependency, so if there is any use of obsolete functions(such as when the installed version of the library is old), shouldn't the optional dependency build fail without affecting other things, also the optional require is used inside rc.js :

var rc = my.require('roboticscape');
, so would these not be necessary to deal with the fail cases you discussed,please let me know if i missed anything

@jadonk jadonk merged commit 3e0e4bb into jadonk:master Jul 27, 2018
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

Successfully merging this pull request may close these issues.

2 participants