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

Porting to NAN #67

Closed
2 of 4 tasks
lwille opened this issue Mar 6, 2015 · 28 comments
Closed
2 of 4 tasks

Porting to NAN #67

lwille opened this issue Mar 6, 2015 · 28 comments
Milestone

Comments

@lwille
Copy link
Owner

lwille commented Mar 6, 2015

The module needs porting to NAN in order to assure compability with previous and future versions of Node and V8.

Development is done in feature/nan.

related issues: #65

Tasks so far:

  • Replace bindings with NAN macros
  • Reimplement camera settings (were removed through removal of cvv8)
  • Refactor async functions to use NanAsyncWorker
  • Use injected (bunyan?) logger instead of logCallback
@lwille lwille added this to the 0.1.8 milestone Mar 6, 2015
lwille pushed a commit that referenced this issue Mar 6, 2015

Verified

This commit was signed with the committer’s verified signature.
keith Keith Smiley
@dubcanada
Copy link

Is feature/nan working? Minus the refactors.

@lwille
Copy link
Owner Author

lwille commented Apr 16, 2015

No, it's not - I didn't even test it yet 😊

On Wed, Apr 15, 2015 at 11:29 PM, dubcanada notifications@github.com
wrote:

Is feature/nan working? Minus the refactors.

Reply to this email directly or view it on GitHub:
#67 (comment)

@dubcanada
Copy link

I am in need of this for a project I am working on. I am debating helping you with the NAN stuff or using node-ffi.

Was there any reason you choose to create C node bindings, rather then using FFI?

@lwille
Copy link
Owner Author

lwille commented May 5, 2015

As libgphoto2 requires some sort of threading, FFI wasn't an option when I started development. Also, I didn't find a way to easily handle structs (now possible with ref-struct).

I just read that node-ffi meanwhile supports async calls and will definitely reconsider it,
especially because it will reduce most of the compilation hassle and ease up development :)

Have you already done some async work with FFI?

Before making such a choice, it would be great to have some proof of concept (open camera, take picture) using FFI.

There's a similar project done in Ruby (https://github.com/zaeleus/ffi-gphoto2).
I guess some boilerplate (structs!) could be generated with ffi-generate (if that's still working)...

You're very welcome to contribute!

@dubcanada
Copy link

I'm going to spend a while over the next two days trying to get a proof of concept working with FFI. I only need open camera, and take photo, and then download photo working for my project so I'll show you that when I get done/working/not working lol

@dubcanada
Copy link

Didn't have a lot of luck with the whole node-ffi. A ton of really weird complicated pointers and things in libgpphoto2.

I'll start playing around with your nan version and try to get that to work. I think that's probably a better solution anyways.

@lwille
Copy link
Owner Author

lwille commented May 14, 2015

Thanks for trying it :)
You'll notice that since removing cvv8, there's no way to retrieve the camera settings. I'd appreciate help on that feature :)

(Just added that to the task list, so we won't forget)

@leroyvi
Copy link

leroyvi commented Jul 3, 2015

Hi lwille

Do you have any news on this subject ? I really like your lib but working with node 0.10 is really annoying.
I will see if I can help but my c++ is very rusty.

@dubcanada
Copy link

It largely works with node 0.10+ already with the nan version just a few
things missing.

On Fri, 3 Jul 2015 6:18 am Nataku notifications@github.com wrote:

Hi lwille

Do you have any news on this subject ? I really like your lib but working
with node 0.10 is really annoying.
I will see if I can help but my c++ is very rusty.


Reply to this email directly or view it on GitHub
#67 (comment).

@sescandell
Copy link

Hi there,

First of all, many thanks for this work. What you've made is so cool!

Just wondering what's the status about Node 0.10+ support?

Any idea when it will be available?

Many thanks,

@elijahparker
Copy link

I just saw this thread now -- I've been working porting my fork to nan the last couple weeks (didn't get very far yet). Where is this at now? I'd love to help out if I can. Thanks!

@geloescht
Copy link

Any news on this? Right now I am trying to use the gphoto2 cli with child_process. It works well, but I guess a proper node module could be more fool-proof.

@lyonlai
Copy link

lyonlai commented Dec 9, 2016

Alright, here it is. I'll give it a try to make it compile again first.

@catesq catesq mentioned this issue Feb 28, 2017
@catesq
Copy link
Collaborator

catesq commented Feb 28, 2017

Sorry about the all-in-one PR. The main changes are:

  • Updated to the current nan.

  • Fixed the gphoto.on('log', callback) method - gphoto.setLoglevel 2 and 3 were so verbose that some messages were being dropped by libuv coalescing events.

  • getConfig is working again (could only find partial examples and unsure if the object it builds has exactly the same structure...).

  • Changed setConfig so it does some basic type conversion and menu/option lookups - especially useful for 'choice' + 'menu' type settings ie on my camera setConfig('capturetarget', 0) now works the same as setConfig('capturetarget', 'Internal RAM').

It passes the tests - apart from a livepreview test that my camera doesn't support. I haven't tried tested it with anything from the examples dir.

@elijahparker
Copy link

@mitnuh: this looks great -- thanks so much! This module has been holding back the node version in my project and I hadn't found the time to tackle this yet, so thank you very much for your work here!

@sorenkrabbe
Copy link

Just wanted to echo the enthusiasm around @mitnuh's contribution. Can't wait to see the PR merged...

lwille added a commit that referenced this issue Mar 4, 2017
@lwille
Copy link
Owner Author

lwille commented Mar 4, 2017

merged #92 into feature/nan. Thank you all, especially @mitnuh, for your contributions and support!

I'd like to ask for some help with testing (still no camera ☹️):

  • Manual tests against current Node LTS with camera on OSX
  • Manual tests against current Node LTS with camera on Linux

Please run the tests and use the files from the examples.
Testing against your own project can be done easily by using npm link:

git clone git@github.com:lwille/node-gphoto2.git && cd node-gphoto2
git checkout feature/nan
npm install
npm link
# cd to your project directory
npm link gphoto2

Sorry, something went wrong.

@IvanDhalluin
Copy link

It's working fine on OSX (node v7.7.1) with livepreview on my project :) (but the example seems to be broken)

@catesq
Copy link
Collaborator

catesq commented Mar 11, 2017

Thanks for testing it Ivan :) the latest pull request seems to get the example working again.

@lwille
Copy link
Owner Author

lwille commented Mar 18, 2017

@mitnuh thanks for merging, please make sure to reference this issue (#67) in your future merges ;-) I'll try to get hold of a camera to test on Linux so we can finally release.

@naeluh
Copy link

naeluh commented May 12, 2017

I have a canon 500d that I am going to test this against in ubuntu 14.04 this weekend, I will let you ifi ti works.
thanks !

@IvanDhalluin
Copy link

It's working perfectly on Ubuntu for me (tested with 2 Canon).

@Sija
Copy link
Collaborator

Sija commented May 12, 2017

Seems like it's GTG, yay! Big thanks to @mitnuh for making this happen, and of course to @lwille for keeping an eye and stewardship of this branch.

@Sija
Copy link
Collaborator

Sija commented Jun 12, 2017

@lwille ping

@rhclayto
Copy link

Any news?

@Sija
Copy link
Collaborator

Sija commented Aug 22, 2017

@lwille need any help with the next releases? I could help you with maintenance if you like.

@Sija
Copy link
Collaborator

Sija commented Dec 1, 2017

@lwille 🏓 There are people waiting for this update for almost 3 yrs now...

@lwille
Copy link
Owner Author

lwille commented Dec 1, 2017

@Sija please go ahead! I currently don't have the resources to actively work on this project.

@Sija Sija modified the milestones: 0.1.8, 0.2.0 Dec 8, 2017
@Sija Sija mentioned this issue Dec 8, 2017
Merged
@lwille lwille closed this as completed in #94 Mar 3, 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

No branches or pull requests