-
Notifications
You must be signed in to change notification settings - Fork 68
Safe shutdown #165
base: develop
Are you sure you want to change the base?
Safe shutdown #165
Conversation
# by Inderpreet Singh | ||
# Adapted by Jake Rye and Bob Davis | ||
|
||
import RPi.GPIO as GPIO |
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.
What if we use Python Periphery, rather than RPi
for GPIO access? This would avoid an explicit RPi dependency. It also happens to be the dependency we will probably use for I2C access if we get the sensors talking directly to the Pi.
A while back, I made a PR to include python-periphery
in rosdep as python-periphery-pip
ros/rosdistro#14364. If we go this route, we can add it to package.xml
as <run_depend>python-periphery-pip</run_depend>
.
WDYT?
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.
That sounds good to me. I'll make the change.
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.
It looks like moving to python-periphery requires the appropriate access to the GPIO files. As a workaround access can be granted for the user running the code.
https://superuser.com/questions/826124/use-sys-class-gpio-in-python-without-root-permissions
In short, we'll need to add these two lines to the install script (something similar):
sudo echo
SUBSYSTEM=="gpio*", PROGRAM="/bin/sh -c \
'chown -R root:gpio /sys/class/gpio && chmod -R 770 /sys/class/gpio;
chown -R root:gpio /sys/devices/virtual/gpio && chmod -R 770 /sys/devices/virtual/gpio'" \ >> /etc/udev/rules.d/99-com.rules
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.
Unfortunately this wasn't as easy as we might have hoped. I found out the permissions error is a timing issue because the gpio pins need to be setup before sending commands to them. I submitted an issue for this on the python-periphery package: here
The other problem is the python-periphery package doesn't have a pull_up feature when setting up a pin to monitor. The RPi.GPIO library allows the pin to be pulled high and then watch when it goes low. The python-periphery package drops the pin low when set up as an input. I don't see how to get around this issue at this time.
I'd recommend we land this change using the RPi.GPIO library and identify another library or fix if we decide to move to different hardware. @gordonbrander Does that sound reasonable?
nodes/signal_shutdown.py
Outdated
def check_for_shutdown(pub13): | ||
# Monitor pin for stable signal to safely shutdown | ||
while not rospy.is_shutdown(): | ||
pub13.publish(GPIO.input(13)) |
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.
Is there a reason to publish this rather than log it? I don't see any subscribers.
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.
Good point. This was mainly put in for debugging and left it thinking it could be useful to have in the future to help debugging. I'll move it over to the logdebug.
nodes/signal_shutdown.py
Outdated
# PIN13 / BCM 27 YELLOW / SW-COM - Signals Shutdown | ||
GPIO.setup(13, GPIO.IN, pull_up_down = GPIO.PUD_UP) | ||
gpio_13 = GPIO(13, "in") # Might need to pull the pin up? |
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.
do we know if this works? Sounds like from the comment that we're not sure.
…p pins correctly. It isn't functional because initiating a pin as low
@rwdavis513 so, from reading the comments it looks like this change will use the RPi library. I'm guessing @gordonbrander's suggestion to use the python-periphery lib was so that we would not have two libs that do similar things. @spaghet is working on the replacement for rosserial right now, and I think it just uses the standard python 'serial' lib and not either RPi or python-periphery (right Rikuo?). So if the best way to get this working is RPi, then I'm for it, if it doesn't conflict with the new work to replace the serial connection to the Arduino. @rwdavis513 how do I wire up my bot so I can test this PR as it is? Thanks! |
This PR shouldn't conflict with the serial patch I'm working on. |
…e/decouple_recipe_persistence Decouple recipe persistence
…e/revert-257-decouple_recipe_persistence Revert "Decouple recipe persistence"
@rbaynes Could you please review and approve this change? Here are the directions to install it. I've tested on several PFCs already and it works correctly. |
Created a node to watch for the safe shutdown signal to come from the power off switch.