Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

url change, new endpoint to plant_list and more timespan options #65

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

alissonzima
Copy link

server_url change, same as pr #62
endpoint for plant_detail changed from PlantDetailAPI to newPlantDetailAPI, retrieving complete date on data dict
added 2 more timespan options, year and all

class Timespan(IntEnum):
   hour = 0
   day = 1
   month = 2
   year = 3
   all = 4

date_str = date.strftime('%Y-%m-%d')
else:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you reorder the if statement to have %Y at the top, %Y-%m after the and %Y-%m-%d at the bottom? Helps with reading through the code faster 🙂


class GrowattApi:
server_url = 'https://server-api.growatt.com/'
server_url = 'https://server.growatt.com/'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server-api.growatt.com is still the URL being used by the app, as not everyone is having issues with the current URL and the new URL might respond differently.
It could cause issues in currently running code.
It's best to overwrite this URL in the scripts using it

Suggested change
server_url = 'https://server.growatt.com/'
server_url = 'https://server-api.growatt.com/'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is outdated, see #77

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants