search

evantahler / nodePhidgets

Node.js JavaScript library to interface with the Phidgets line of hardware boards.
Other
29 stars 8 forks source link

Add support for PhidgetTemperatureSensor boards #22

Closed bergera closed 7 years ago

bergera commented 7 years ago

I've successfully tested this with a 1048_0 - PhidgetTemperatureSensor 4-input.

I did my best to follow the existing style (which is nice and clean, thanks for that), let me know if there's anything I can change.

djipco commented 7 years ago

Awesome! If you have more, keep them coming. Thanks!

djipco commented 7 years ago

I made a few cosmetic changes and published the new version (0.5.7) on npmjs.com. Thanks again!

djipco commented 7 years ago

I'm thinking the setTemperatureChangeTrigger() method would probably be better named setTemperatureChangeThreshold(), setTemperatureThreshold() or, given that it's a temperature sensor, simply setThreshold() What do you think?

bergera commented 7 years ago

TemperatureChangeTrigger is defined in http://www.phidgets.com/docs/1048_User_Guide#API as well as the C header files for libphidget21, while the keyword expected as input and given as output from the Web service is just Trigger, and in the Phidget Control Panel GUI the label is "Sensitivity".

My feeling personally is that it should be as close to the "official" documentation as possible (that is, left as setTemperatureChangeTrigger), even when that documentation isn't consistent. But with all those differing names, it could be whatever you think is most appropriate and it wouldn't be without precedent ;).

djipco commented 7 years ago

Yeah... their API and documentation is not the most consistent. For me, trigger, sensitivity and threshold are 3 different things but I guess we can stick to setTemperatureChangeTrigger all things considered.

bergera commented 7 years ago

I think the README could be reorganized to more directly point potential users to the generated API docs, which are pretty clear on the intention of the methods. And it's a pretty simple class, with only two methods and three events.

bergera commented 7 years ago

And yeah, threshold would be my preferred terminology in a perfect world, too.

djipco commented 7 years ago

Sure. What do you have in mind regarding the README?

bergera commented 7 years ago

The repo's README is mostly directed at the PhidgetInterfaceKit, with the other devices briefly mentioned at the end. I had already written most of the PhidgetTemperatureSensor code before I realized there was link at the bottom to the online docs.

I would move any detailed device-specific examples into the inline YUIDoc blocks for each device, and add links for each device directly to the API doc page.

I can take a swing at it tomorrow if you'd like.

djipco commented 7 years ago

Oh really? I never realized I had burrowed the API doc so deep. If you'd like to take a swing at it, that would be great!

bergera commented 7 years ago

Sure, I'll try to have something worked out this week.

For that matter, the API docs provided within the repo under docs/ seem to be pretty out of date.

djipco commented 7 years ago

You're so right about the docs folder. Something was broken in the Gruntfile. I fixed it.