andig / pimatic-fritz

Pimatic plugin for Fritz!Box SmartHome and FritzDect!200 Node
GNU General Public License v2.0
8 stars 6 forks source link

Revised implementation of enhancement: add temp reading of fritz box … #11

Closed mwittig closed 8 years ago

mwittig commented 9 years ago

…#2, includes fixes for: No values anymore after update #7

andig commented 9 years ago

Are you confident now that this works for everybody? Are the fixes from https://github.com/andig/pimatic-fritz/pull/10/files no longer needed?

mwittig commented 9 years ago

I have tested both update modes thoroughly. Indentation changes have no impact on semantics (see comments on diff). Code changes of PR#10 are included in the current PR.

andig commented 9 years ago

Could you move the PR10 changes to smartfritz? As it helps abstacting the basic fb api I believe thats were it belongs. Fine to merge then. As for indentation I prefer it unchanged if code is not touched but I realize that IDEs may have differing options.

mwittig commented 9 years ago

@andig happy to do that eventually, but I don't have much time right now. So, it'll be great if you can accept the PR as is for now. Regarding smartfritz I am wondering whether or not @nischelwitzer is still maintaining smartfritz. Did you consider contributing your code changes back to the original repo?

andig commented 9 years ago

Regarding smartfritz I am wondering whether or not @nischelwitzer is still maintaining smartfritz. Did you consider contributing your code changes back to the original repo?

Tried to, but never got a response: https://github.com/nischelwitzer/smartfritz/issues/1. As smartfritz-promise has a different api it doesn't make much sense anyway.

happy to do that eventually, but I don't have much time right now. So, it'll be great if you can accept the PR as is for now.

Same for me... Please move the one function to https://github.com/andig/smartfritz

andig commented 9 years ago

ping @mwittig

mwittig commented 9 years ago

Ah, ok. Sorry for not looking into this any earlier. Hope, I find some time to get this done next week.

andig commented 8 years ago

I've added a TemperatureSensor based on the XML api that will be able to read temps of both switches and thermostats. This PR is no longer needed.