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

[PROPOSAL] Fixing udev rules #27

Open
knghtbrd opened this issue Apr 17, 2016 · 2 comments
Open

[PROPOSAL] Fixing udev rules #27

knghtbrd opened this issue Apr 17, 2016 · 2 comments

Comments

@knghtbrd
Copy link
Member

Proposal for @RasppleII/owners (and anyone else interested)

Currently we try to do clever things with the Raspberry Pi to assign certain USB serial devices to certain functions. If you don't speak Linux geek, I'll explain in a moment--if you do, here's the script that generates the current udev rules file:

if [[ ! -f /etc/udev/rules.d/50-usb.rules ]]; then
    echo "A2CLOUD: Creating device rules for USB ports..."
    udevLines=
    if [[ $isRpi ]]; then
        # assign ttyUSBupper, or ttyUSBupper_hubXX, for shell usb-to-serial adapter
        # assign ttyUSBlower, or ttyUSBlower_hubXX, for ADTPro usb-to-serial adapter
        # (A/A+ direct attach is always ttyUSBlower;
        #   hub attached to A/A+ will be ttyUSBupper on port 2, and ttyUSBlower on port 3)
        udevLines+='KERNEL=="ttyUSB*", KERNELS=="1-1:1.0", SYMLINK+="ttyUSBlower", RUN+="/usr/local/sbin/ttyusbhandler add ttyUSBlower"\n'
        udevLines+='ACTION=="remove",  ENV{DEVPATH}=="*1-1:1.0*", RUN+="/usr/local/sbin/ttyusbhandler remove ttyUSBlower"\n'
        udevLines+='KERNEL=="ttyUSB*", KERNELS=="1-1.2:1.0", SYMLINK+="ttyUSBupper", RUN+="/usr/local/sbin/ttyusbhandler add ttyUSBupper"\n'
        udevLines+='ACTION=="remove",  ENV{DEVPATH}=="*1-1.2:1.0*", RUN+="/usr/local/sbin/ttyusbhandler remove ttyUSBupper"\n'
        udevLines+='KERNEL=="ttyUSB*", KERNELS=="1-1.3:1.0", SYMLINK+="ttyUSBlower", RUN+="/usr/local/sbin/ttyusbhandler add ttyUSBlower"\n'
        udevLines+='ACTION=="remove",  ENV{DEVPATH}=="*1-1.3:1.0*", RUN+="/usr/local/sbin/ttyusbhandler remove ttyUSBlower"\n'
        for i in {1..25}; do
            ii=$(printf %02d $i)
            udevLines+='KERNEL=="ttyUSB*", KERNELS=="1-1.2.'$i':1.0", SYMLINK+="ttyUSBupper_hub'$ii'", RUN+="/usr/local/sbin/ttyusbhandler add ttyUSBupper_hub'$ii'"\n'
            udevLines+='ACTION=="remove",  ENV{DEVPATH}=="*1-1.2.'$i':1.0*", RUN+="/usr/local/sbin/ttyusbhandler remove ttyUSBupper_hub'$ii'"\n'
            udevLines+='KERNEL=="ttyUSB*", KERNELS=="1-1.3.'$i':1.0", SYMLINK+="ttyUSBlower_hub'$ii'", RUN+="/usr/local/sbin/ttyusbhandler add ttyUSBlower_hub'$ii'"\n'
            udevLines+='ACTION=="remove",  ENV{DEVPATH}=="*1-1.3.'$i':1.0*", RUN+="/usr/local/sbin/ttyusbhandler remove ttyUSBlower_hub'$ii'"\n'
        done
    else
        # on non-Pi installations, assign ttyUSBupper to ttyUSB0 and ttyUSBlower to ttyUSB1
        udevLines+='KERNEL=="ttyUSB0", SYMLINK+="ttyUSBupper", RUN+="/usr/local/sbin/ttyusbhandler add ttyUSBupper"\n'
        udevLines+='ACTION=="remove",  ENV{DEVPATH}=="*ttyUSB0*", RUN+="/usr/local/sbin/ttyusbhandler remove ttyUSBupper"\n'
        udevLines+='KERNEL=="ttyUSB1", SYMLINK+="ttyUSBlower", RUN+="/usr/local/sbin/ttyusbhandler add ttyUSBlower"\n'
        udevLines+='ACTION=="remove",  ENV{DEVPATH}=="*ttyUSB1*", RUN+="/usr/local/sbin/ttyusbhandler remove ttyUSBlower"\n'
    fi
    echo -e "$udevLines" | sudo tee /etc/udev/rules.d/50-usb.rules >/dev/null
else
    echo "A2CLOUD: Device rules for USB ports already exist."
fi

That spits out this 106 line file on Raspberry Pi (only 4 lines on other systems, but you probably need to reconfigure it yourself in that case). What those lines do is create one or two theoretically human-readable aliases to your USB to serial devices, based on where they're plugged in. See, normally Linux names the first one it sees ttyUSB0, and the second ttyUSB1, etc. The problem with USB is that if you boot the machine up with the devices already plugged in, they'll be in a certain order. It might not be the same order it would've been if you plugged them into a running system.

Linux has some "persistent name" rules already written, but they're not reliable because it's stupidly possible to have two indistinguishable USB-serial devices. In fact not only is it possible, but with most cheap serial dongles based on counterfeit Prolific chips or those "quality" Keyspan devices all of us Mac users have laying around somewhere, it's almost guaranteed.

@IvanExpert solved this by declaring the top USB port (on the original Raspberry Pi model B) was for serial console, the bottom was for ADTPro, and the GPIO serial was for the Apple II Pi card. The model A would be the "lower" port if you didn't have a hub plugged in. And that was perfectly logical until the model B+ came out with four ports, the ODROIDs and Banana Pis came out with arbitrary USB configurations, and of course none of this applies to people running A2CLOUD on Intel machines or under VMs.

Assuming all of that isn't a factor, the current rules will create ttyUSBupper OR ttyUSBupper_hub## for devices found on a hub. Usually there's only one of these, but there might not be, and if there's more than one you need a script to determine which of those to use. It turns out that ttyusbhandler has that logic, but a new standard Linux thingy called systemd does not. And that breaks stuff in irritating ways.

So we need new rules that always use one name! And while we're at it, can we possibly make the one name something useful? Yes we can!

#98-a2cloud.rules

# Information about how to configure serial ports manually here.
# Nutshell version is uncomment and replace KERNEL= with your match
# condition.  Add some mechanism to let the user modify this part of
# the file and let us blow the rest away...
#
#KERNEL="ttyUSB0" \
#  TEST!="/dev/ttyADTPro" SYMLINK+="ttyADTPro" GOTO="a2cloud_devices_end"
#KERNEL="ttyUSB1" \
#  TEST!="/dev/ttyConsole" SYMLINK+="ttyConsole" GOTO="a2cloud_devices_end"

# Default serial ports for ADTPro and serial console
# # Please see <url> for details
SUBSYSTEM="tty" KERNELS=="1-1:1.0" TEST!="/dev/ttyADTPro" SYMLINK+="ttyADTPro"
SUBSYSTEM="tty" KERNELS=="1-1.3:1.0" TEST!="/dev/ttyADTPro" SYMLINK+="ttyADTPro"
SUBSYSTEM="tty" KERNELS=="1-1.3.*:1.0" TEST!="/dev/ttyADTPro" SYMLINK+="ttyADTPro"
SUBSYSTEM="tty" KERNELS=="1-1.2:1.0" TEST!="/dev/ttyConsole" SYMLINK+="ttyConsole"
SUBSYSTEM="tty" KERNELS=="1-1.2.*:1.0" TEST!="/dev/ttyConsole" SYMLINK+="ttyConsole"

# Prevent conflicts with ttyADTPro==ttyConsole
SYMLINK=="ttyADTPro" SYMLINK=="ttyConsole" SYMLINK=-"ttyADTPro"
LABEL="a2cloud_devices_end"

# Run A2CLOUD services when devices are added
ACTION=="add" SYMLINK=="ttyADTPro" RUN+="/usr/local/sbin/ttyusbhandler remove ttyADTPro"
ACTION=="remove" ENV{DEVLINKS}=="/dev/ttyADTPro" RUN+="/usr/local/sbin/ttyusbhandler remove ttyADTPro"
ACTION=="add" SYMLINK=="ttyADTPro" RUN+="/usr/local/sbin/ttyusbhandler remove ttyADTPro"
ACTION=="remove" ENV{DEVLINKS}=="/dev/ttyADTPro" RUN+="/usr/local/sbin/ttyusbhandler remove ttyADTPro"

This is still a work in progress and so far it addresses only the Raspberry Pi case for now. Let's start in the second major blob for explanations. The "KERNELS" bit is udev voodoo that specifies the USB bus and where on it the serial device lives. SYMLINK+= adds an alias to whatever the system wants to call the device to the name we want to use for it, either ttyADTPro or ttyConsole. The TEST!= bit is there to make sure that if the alias already exists, we don't try to overwrite it. I'm not 100% sure that's necessary in udev, but better safe than sorry until I do know.

Next, the possibility exists that a device might somehow get assigned both the symlinks for ttyADTPro and ttyConsole. That's a no-no and will break A2CLOUD. It doesn't happen with the rules above as written, but if you start adding your own, it might happen. So if it does, we use it as a console so that you can hopefully use the console if you need it to fix the problem. :)

Next are the rules for starting and stopping things. We're still using ttyusbhandler for that, though its logic would become a bit simpler.

That leaves the old rules. I think we probably ought to leave them there if they exist. They're a "dropping", but we don't know if they're used in any config or not. Best to just make ttyusbhandler silently ignore requests using those names.

Thoughts?

@knghtbrd
Copy link
Member Author

Improvement:

# 98-a2cloud.rules

# On the Raspberry Pi, we default to assigning specific functions USB
# serial devices connected to specific ports.  If you wish to configure
# your devices manually on the Raspberry Pi, comment out the following
# line to use the specific device configuration below
TEST=="/usr/bin/raspi-config" GOTO="a2cloud_rpi_start"


# Specific device configuration
# 
# On systems other than the Raspberry Pi, we cannot predict how your
# USB devices are arranged or where they are connected.  We default to
# using ttyUSB0 for ADTPro and ttyUSB1 for serial console, but you can
# (and probably you will) want to change that.
#
# FIXME: Instructions how would be nice...

KERNEL="ttyUSB0" \
    TEST!="/dev/ttyADTPro" SYMLINK+="ttyADTPro"

KERNEL="ttyUSB1" \
    TEST!="/dev/ttyConsole" SYMLINK+="ttyConsole"

GOTO="a2cloud_rpi_end"


# Raspberry Pi default device configuration
#
# Describe this and please see <url> for details FIXME
LABEL="a2cloud_rpi_start"
SUBSYSTEM="tty" KERNELS=="1-1:1.0" TEST!="/dev/ttyADTPro" SYMLINK+="ttyADTPro"
SUBSYSTEM="tty" KERNELS=="1-1.3:1.0" TEST!="/dev/ttyADTPro" SYMLINK+="ttyADTPro"
SUBSYSTEM="tty" KERNELS=="1-1.3.*:1.0" TEST!="/dev/ttyADTPro" SYMLINK+="ttyADTPro"
SUBSYSTEM="tty" KERNELS=="1-1.2:1.0" TEST!="/dev/ttyConsole" SYMLINK+="ttyConsole"
SUBSYSTEM="tty" KERNELS=="1-1.2.*:1.0" TEST!="/dev/ttyConsole" SYMLINK+="ttyConsole"
LABEL="a2cloud_rpi_end"

# Prevent conflicts with ttyADTPro==ttyConsole
SYMLINK=="ttyADTPro" SYMLINK=="ttyConsole" SYMLINK=-"ttyADTPro"

# Run A2CLOUD services when devices are added
ACTION=="add" SYMLINK=="ttyADTPro" RUN+="/usr/local/sbin/ttyusbhandler remove ttyADTPro"
ACTION=="remove" ENV{DEVLINKS}=="/dev/ttyADTPro" RUN+="/usr/local/sbin/ttyusbhandler remove ttyADTPro"
ACTION=="add" SYMLINK=="ttyConsole" RUN+="/usr/local/sbin/ttyusbhandler remove ttyConsole"
ACTION=="remove" ENV{DEVLINKS}=="/dev/ttyConsole" RUN+="/usr/local/sbin/ttyusbhandler remove ttyConsole"

The manual configuration rules are needed for non-Raspberry Pi systems regardless, so let's take advantage of the de facto test for Raspbian to bypass the rules for other systems. On the Pi, we do our magic bus parsing by default so it just works on headless systems as documented. That documentation needs updating both here and on the website, but that's fine.

Some copypasta errors were introduced with the above file since I moved it from an email to a working file on another machine and then again into a web browser for filing the issue. This version fixes those issues.

Still a work in progress, but a better place to begin discussion.

@knghtbrd
Copy link
Member Author

The current beta does not have this functionality, and I am not willing to hold up the release to implement it. I remain somewhat unsure that this is the most correct solution for resolving this issue, and I do not know where best to get help determining first of all if this solution is on the right track or if it can be improved upon.

We now use systemd, but I do intend to maintain backward-compatibility with sysvinit if possible. I realize that it may not be, in which case I would consider a systemd-requiring solution appropriate. I am marking this one as wanting help for now.

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

No branches or pull requests

1 participant