search

ady624 / webCoRE

webCoRE is a web version of CoRE
GNU General Public License v3.0
251 stars 984 forks source link

new hubInfoHandler method to replace non-functional hubUpdatedHandler #99

Closed bjones14 closed 4 years ago

bjones14 commented 4 years ago

The hubPowerSource updates were not working for me, so I decided to investigate. I found that the latest version of the code had a small bug (with the assumed inadvertent question mark in the method call), as well as the incorrect event handler setup.

There was no "HubUpdated" event to subscribe to, which is why the hubPowerSource value change updates were never triggering. Instead, there was a hubInfo event to subscribe to, which contains a key named "batterygpiostat" which gives the hub power source information.

I have implemented and tested this with my hub. I am not familiar with how different hubs versions are supported (if they are), I just know this is what was required for my hub to function. A quick google search told me that other people were experiencing the same issue as well.

idpaterson commented 4 years ago

Thanks! In case it is relevant, what is your hub version?

bjones14 commented 4 years ago

@idpaterson I have a "hub v2, US customer Rev E" hub and the firmware version is 000.028.00012

idpaterson commented 4 years ago

Thanks for the change, with a small tweak this will go into the next update. Two comments on the code for this one:

  1. The ? in getHub()?.isBatteryInUse() was probably intentional; removing it will cause an error if getHub() does not return a value.
  2. I can't find any references to a "HubUpdated" event past 2017, but it doesn't look like there is any harm in leaving the old code in case it is still relevant for v1 hubs or similar. Would you please update this to subscribe to and handle both events?
bjones14 commented 4 years ago

Thanks for the change, with a small tweak this will go into the next update. Two comments on the code for this one:

1. The `?` in `getHub()?.isBatteryInUse()` was probably intentional; removing it will cause an error if `getHub()` does not return a value.

2. I can't find any references to a "HubUpdated" event past 2017, but it doesn't look like there is any harm in leaving the old code in case it is still relevant for v1 hubs or similar. Would you please update this to subscribe to and handle both events?

  1. Understood! This is my first run-in with Groovy so I wasn't sure if it was a syntax quirk I was unaware of.

  2. Good point, I will make the changes shortly here and update this PR