-
Notifications
You must be signed in to change notification settings - Fork 69
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
Update README.md with updated PW3 instructions #493
Conversation
Added additional information from issue jasonacox#492
README.md
Outdated
* copy of pypowerwall.env.sample and rename it pypowerwall.env | ||
* Update file contents to include PW_GW_PWD paratmeter the value being your Tesla Powerwall3 wifi password. | ||
* Update PW_EMAIL and PW_PASSWORD to be empty. | ||
* Save and run ./setups.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, THANK YOU for opening a PR.
Now, as to this - you should not need to do this. The first thing setup.sh does it copy that file and creates the PW_GW_PWD entry if you select PW3 mode, and then create the empty PW_EMAIL and PW_PASSWORD fields.
I think setup.sh failed for you because the curl
command used to detect the gateway failed. I need to add more error handling to the setup.sh that basically says, "I didn't find the Powerwall GW, do you want to retry?" If you say no, it should continue as if the GW works and set up all the above you list out.
Having said that, I do think this data would be good to include in the Troubleshooting section for PW3, including the working evn file example below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I agree with you, moving the steps to troubleshooting is a good call especially since I redeployed the dashboard in a fresh W11 VM and didn't run into the issue. The setup script actually discovered my PW3 so I was impressed.
Maybe on the retry, you do it internally 3 times, if it fails play a message to check networking or continue as it was working. I had about 25% of pings go through so maybe that's worth it.
Here is a not clean version of the test_ip() function that increases the timeouts for 2nd an 3rd as well. I'm sure a loop would look cleaner but may suffice. I may have mis-read mis-understood the code but this is what I was thinking when I saw the test_ip function.
function test_ip() {
local IP=$1
if [ -z "${IP}" ]; then
return 1
fi
if curl -k --head --connect-timeout 1 --silent https://${IP} > /dev/null 2>&1; then
if curl -k --head --connect-timeout 2 --silent https://${IP} > /dev/null 2>&1; then
if curl -k --head --connect-timeout 2 --silent https://${IP} > /dev/null 2>&1; then
return 0
else
return 1
else
return 1
else
return 1
fi
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic here is a bit backward. The retry curl will need to be in the else branch (return = 0 is consider successful run in bash, which I understand seems backward). However, my issue with doing this is that if your host is having trouble running curl
against the gateway, then pypowerwall will as well. I would rather detect that issue early so that the system owner can investigate and fix it.
I made some changes to your PR to have the setup.sh
provide more information to the user. It also occurs to me that PW3 owners may select option 1 (local mode) and not know that the PW3 can't support the hybrid
data. The script now asks the user if they have a Powerwall 3 if it detects the GW and will erase PW_PASSWORD and PW_EMAIL to ensure pypowerwall comes up in full
mode.
echo "" | ||
exit 1 | ||
fi | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm combining an enhancement based on #494 to help with installs where the user does not have write access to their local directory.
@@ -1 +1 @@ | |||
4.4.4 | |||
4.4.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This updates us to the next version.
Updated instructions specific to successful Powerwall 3 integration based on issue #492