search

nathankellenicki / node-poweredup

A Javascript module to interface with LEGO Powered Up components.
https://nathankellenicki.github.io/node-poweredup/
MIT License
486 stars 62 forks source link

current reporting disabled? #23

Closed stabenau closed 5 years ago

stabenau commented 5 years ago

Hello Nathan,

https://github.com/nathankellenicki/node-poweredup/blob/2e6d2b0dbc117c03379cf429feb100d3c6adabb4/src/hub.ts#L102

The hub's current (milliamps) reporting used to work for an older version and seems now to be disabled. Any particular reason?

Best regards,

Arne

nathankellenicki commented 5 years ago

Hi Arne,

The code for current detection was always dubious at best. Despite the efforts of myself and a few others, we were never quite able to figure out the math that Lego uses for the current value from the raw data, or even if this sensor represented current at all.

Especially as different hubs report wildly different values for the same sensor, so I was unable to make heads nor tails about it.

I disabled it shortly after Lego released the Lego Wireless Protocol 3.0 documentation as despite the mention of current in there, they don't go into any detail about it, so I was extra skeptical about the sensor actually reporting current information.

Did you find it useful? What was your use case? If you found it useful, I'm happy to put it back in - just be aware that it may not work as advertised! :)

stabenau commented 5 years ago

Dear Nathan,

Thanks for your answer. I would agree with you, there is no good reason to have a current function that doesn't actually report a current. However, I have here a piece of software that uses the current for a bit of UI eye candy, so Im inclined to put it back in locally. Unfortunately my knowledge of npm basically ends at npm install ... I'll try to badger google and stack-overflow into telling me, how to modify and rebuild modules for npm, but if you could drop me a pointer, it would probably be much easier for me.

I tried to uncomment the section in hub.js in dist subdir, but this alone did not do the trick.

Regardless, thanks for this module, its a bit like magic to make the trains go without the remote control :-)

All the best and if you are into it, Happy Easter!

Arne

nathankellenicki commented 5 years ago

Thank you, and to you!

I haven't had a chance to check out the code yet, but uncommenting that block may not be enough - I might have to put some message parsing back in.

I think a reasonable approach may be to put the functionality back in, but leave it undocumented. Perhaps a comment describing the issues, if someone were to find it in the code. :)

I'll have a look today/tomorrow and let you know when it's back.

nathankellenicki commented 5 years ago

Apologies for taking longer than expected - I've added the current back in. :)

I'm going to be doing more testing asap to see if I can figure out the voltage/current some more, but in the meantime, this'll get you the raw value you were using again.

nathankellenicki commented 5 years ago

Hi @stabenau, with the help of @undera, I've managed to figure out the correct voltage/current values for the hubs. :) I've published v2.5.1 which includes the correct voltage/current reporting.

Note that I've discovered the Powered Up remote does not report current, only voltage.

stabenau commented 5 years ago

Hello Nathan,

Thanks a lot for the fix / feature and generally your project!

All the best,

Arne