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

Replace startup scripts with one service file #38

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

Conversation

MrSpoon
Copy link

@MrSpoon MrSpoon commented Apr 20, 2021

This pull request replaces the current startup scripts with a single service file. This new service file includes several new features making it easier to control the controller and better error handling.

  • Stopping the controller will also stop all forked commands
  • On unclean exit, script will attempt to restart
  • Restarting will stop should too many restarts occur in a short period
  • TTS startup announcements delayed until after controller connects to server

@SkyeOfBreeze
Copy link

Although this may be better for most, it feels like both options should remain viable, with this being the recommended option? While it may be rare, some OSes don't have the service functionality, such as linux in windows, which probably won't usually get used though.

In those cases though, the python script could be run directly though, unless this breaks that?

@Nocturnal42
Copy link
Collaborator

Nocturnal42 commented Apr 20, 2021

Looks good, though I have only had a quick glance through. You do need to add a tts call to networking.py to report the internet has connected / reconnected. Since the main job of that function is to report on the up and downs of the internet connection to the robot owner.

I also agree with Brendon. I have no issue with making it service based as the default, but the scripts should remain for the situations where a serviced based startup is either not available or not desirable. I would suggest moving them into an 'old' subfolder with a readme explaining their use was depreciated and not compatible with a service based startup.

Edit: Actually, now that I think more about this. They really should not be moved or deleted. That would be a breaking change. It would break the setup of any existing robot that does a git pull (or uses the experimental update code in the controller).

Restore deleted files to maintain backwards compatibility.
Misread the problem on issue remotv#20. It was not an issue with the tts but with a continuous restarting loop due to api authentication failure.
@MrSpoon
Copy link
Author

MrSpoon commented Apr 20, 2021

I see. Backwards compatibility is a must and the deleted files have been restored. I've also moved back the tts startup message to it's original location. I had misread the problem on issue #20. It was not an issue with the tts but with a continuous restarting loop due to api authentication failure.

When the script exits cleanly, status code 0, the service file will not attempt a restart. So when a api authentication fails the controller exits with status code 0 and the controller will not restart, avoiding tts spam.

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