search

netmanchris / pyuhooair

Python Wrapper for uHoo Air Quality Sensor REST API
Apache License 2.0
17 stars 6 forks source link

Adding temperature data, as this was missed. #1

Closed mcclown closed 5 years ago

mcclown commented 5 years ago

I've added the ability to retrieve the temperature data from the UhooDev class, as this looks to have been missed.

mcclown commented 5 years ago

The tests have failed. It looks like the email address and password are missing from the repo config on Travis CI. I assume these were previously setup to point at your account?

netmanchris commented 5 years ago

@mcclown Hey Stephen! Thanks for the PR. TravisCI is setup with a secret token to be able to access my uHoo devices. I’ll try and dig into this later this week, but as a quick comment, it looks like you modified the library file but didn’t add a corresponding test. I need to get back to this but goal is to have coverage above 90%.

Seems like it’s probably a legitimate fail so you might want to run the tests against your copy of the library and see where it’s failing.

Post more once I’ve had a chance to open this up and take a look.

mcclown commented 5 years ago

No problem.

I was going to add tests but there was no tests for the UhooDev class at all yet. I wrote some that I used locally to verify it but they're using pytest. I see you're using unittest/nose but I can submit my pytest code as well if you want.

mcclown commented 5 years ago

Hey, any update on this? Anything else I can assist with, to get this included?

netmanchris commented 5 years ago

Hi @mcclown

My apologies. Not even going to lie. This totally slipped out of my head. I just updated the library and also pushed a new version (0.0.2) to pypi with the temperature added to the class object. I'm going to see if I can spend a bit of time on this library this week to add in the missing tests and fix some of the broken tests. It looks like the uHoo API might have changed in the months since I originally wrote this. ( or I just never got working tests... at this point I'm willing to believe either one... ) Really appreciate you bring this back up to me. I am going to be using nose tests for all the testing for this library. So no need to PR the pytest code. Let me know if the code is working for you now. I actually saw your message and just fixed it myuself before coming to GITHUB and realizing there was a PR. nothing at all against your code :)

mcclown commented 5 years ago

No problem, I'd forgotten about it as well. I just had to update the integration I used it in last night so it brought it back into my mind again. Thanks for resolving this.