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

TEDAPI network route script #519

Closed
jasonacox opened this issue Sep 12, 2024 · 8 comments
Closed

TEDAPI network route script #519

jasonacox opened this issue Sep 12, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@jasonacox
Copy link
Owner

jasonacox commented Sep 12, 2024

Add network route setup script to help Dashboard users connect with TEDAPI gateway endpoint.

@jasonacox : attached the script I used to create the TDAPI network routing script and add its cron task. Played with it on my sandbox, and today got a chance to upgrade Powerwall Dashboard on my production system, so tried it there. Worked fine! Note changed extension from .sh to .txt so GitHub issues would accept it.
TDAPI_network.txt

Originally posted by @SCHibbard in #387 (comment)

@jasonacox jasonacox added the enhancement New feature or request label Sep 12, 2024
@jasonacox
Copy link
Owner Author

Hi @SCHibbard - I created this issue to track the enhancement and work through details. First pass, it looks good, but I would remove the requirement for the user to run setup.sh before running this script. Ideally someone would be able to run this first (eventually even have setup.sh run it), get the 192.168.91.1 route in place and setup.sh would detect and use that when it runs.

Would need to address these parts...

if [ ! -f pypowerwall.env ]; then
    echo "ERROR: Missing pypowerwall.env file. This must run from Powerwall Dashboard installation directory."
    echo ""
    exit 1
fi

...  

if ! $(docker ps | grep -qw "pypowerwall"); then
	echo "Docker is not running, or pypowerwall docker container is not running."
	echo "Run Powerwall Dashboard setup before running this script."
	exit 1
fi

....

docker exec -it pypowerwall python3 -m pypowerwall scan -ip=${IP}

Do you want to submit this as a pull request? GitHub will add you as a contributor to the project. :) Details on how to do that:

  1. Fork the project
  2. Create the script (maybe we call it add_route.sh ?) in your repo and commit it to your repo.
  3. Perform a pull request in github's web interface.

Otherwise, let me know and I'll add it.

@SCHibbard
Copy link
Contributor

SCHibbard commented Sep 12, 2024

@jasonacox: Wondered about starting a new issue, thanks. I can do a pull request this weekend. As far as requiring Powerwall Dashboard to be up and running, my only reason was to put the generated script in the same folder (and to steal your network scan function!) I had originally placed it in /etc, but changed my mind. You see a problem putting it there? Better place?

@jasonacox
Copy link
Owner Author

It would be nice to allow the user to run optionally run add_route.sh script before running setup.sh so that it detect the TEDPAI endpoint. Let's see if there is another way to do the scan... I'll take a look too.

@SCHibbard
Copy link
Contributor

Slowly getting back to things after walking a 1.2M ft^2 trade show for a week. I understand now why you'd like add_route.sh run before setup.sh is called. I did not try a total cold install (remove Powerwall Dashboard), so I didn't catch the opportunity to have it in place when setup.sh goes hunting for the Powerwalls. And I guess /etc is the best place to locate the resulting script cron runs. Requires sudo as well, but the user will only be asked for credentials once.

@jasonacox
Copy link
Owner Author

Sorry, I didn't comment on the script location. From a security point of view, I agree with you that the resulting script that root's crontab runs needs to be a protected file that only root can edit (sudo). If our project later makes enhancements or fixes to the add_route.sh tool that changes that resulting script, we can tell the user in the upgrade.sh script to re-run the add_sroute.sh to get the cron script updated.

However, giving nod to https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard I think it is probably best to avoid putting executable scripts in /etc and putting those in /root or /root/scripts instead. I suspect there are some packages that do put executables in /etc but that folder is typically reserved for configuration data or OS specific package manager configuration scripts. Using /root or /root/scripts as the location will also make it transparent to the system owner/admin that this is a root level cron task (we might even add that as a comment to the script).

@SCHibbard
Copy link
Contributor

Thanks, should have checked. My Unix filesystem hierarchy experience dates back to the late 70s, I think backed then we stuffed lots of things in /etc without regard to organization.

@SCHibbard
Copy link
Contributor

SCHibbard commented Sep 14, 2024

Script modified and waiting for you! Few points:

  1. Removed the scan function completely for now, we could put back in when one of us finds a clever way.
  2. Placed cron task script in /root/scripts.
  3. Commented out the logging function in the created script, as I suspect it will rarely be needed. Can be uncommented if required.

@jasonacox
Copy link
Owner Author

Closing this with the release.

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

No branches or pull requests

2 participants