Add getTemp#2
Conversation
|
@Morgul thanks for putting this together, this'll be great to add. I had a few comments, not quite sure how best to handle the units — what you did isn't bad but I can't think of much better, lmk your thoughts. |
|
@brendanashworth I'll get those in tomorrow, shouldn't take too long. Just have to say, this was my first foray into node bindings, and the first C code I've written in like 5 years. It was honestly a lot of fun, and the library was pretty simple to get started with. It's currently powering my node fan controller for my poor rMBP which a broken temperature sensor. (The CPU die sensors work, but the proximity sensor gives really wrong readings, and since that controls the fans... I had to improvise.) |
|
@Morgul no rush! Happy to see someone actually uses this project 👍 open source is a wonder. |
|
@brendanashworth Should be good to go now. |
brendanashworth
left a comment
brendanashworth
left a comment
There was a problem hiding this comment.
just a few last things. CI had an issue due to the tests
| t.ok(typeof info.name === 'string'); | ||
| t.ok(typeof info.speed === 'number'); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Can we test for type checking? (passing non-strings, etc)
|
@brendanashworth Got those fixes in. Let me know if there's anything else. |
|
@brendanashworth I looked at those failures, and they don't seem related to my changes. It seems like incompatibility with NAN and node < 4.X. |
2 participants
Exposed the
get_tmpcall asgetTemp. Supports units (as strings), and includes both documentation and unit tests.