kbr / fritzconnection

Python-Tool to communicate with the AVM Fritz!Box by the TR-064 protocol and the AHA-HTTP-Interface
MIT License
304 stars 59 forks source link

Uptime from DeviceInfo instead of WANIPConn #101

Closed chemelli74 closed 2 years ago

chemelli74 commented 3 years ago

Porting custom integration to a core module in HomeAssistant, I noticed that fritzconnection code acalculate device uptime using WANIPConn/GetStatusInfo/NewUpTimeinstead of DeviceInfo/NewUpTime (ref. https://avm.de/fileadmin/user_upload/Global/Service/Schnittstellen/deviceinfoSCPD.pdf)

Would you accept a PR to change that ?

Simone

kbr commented 3 years ago

As far as I can see these are different uptimes: WANIPConn / GetStatusInfo/ NewUptime returns the uptime of the internet-connection, as the DeviceInfo / GetInfo / NewUpTime returns the uptime of the hardware. So I would not change this, but it would be ok to add a new method to FritzStatus. However the user should know that this call also transfers the NewDeviceLog with potentially a lot of data.

What do you think about?

chemelli74 commented 3 years ago

I agree those are two different type of uptime. So, while adding the device uptime, please rename also the WAN uptime so to make clear what is what.

I don't get why you think that calling NewUpTime action with all do also a NewDeviceLog call.

Simone

kbr commented 3 years ago

Renaming an api call is backward incompatible and should get avoided. But a new property could have the name i.e. hardware_uptime.

Calling the DeviceInfo / GetInfo action will return all out-arguments, not just the NewUpTime.

chemelli74 commented 3 years ago

What I would do if I were you is:

Simone

kbr commented 3 years ago

If you need the router system uptime you can use the call_action method directly in your application and don't have to wait for an update. If you like to have a dedicated method for this in the FritzStatus module feel free to provide a pull request.

chemelli74 commented 2 years ago

PR ready for merging: https://github.com/kbr/fritzconnection/pull/104

Simone

kbr commented 2 years ago

merged.