sl-modem-daemon init.d script looks for information about ALSA modem in wrong place

I have to correct my statement in this bug report :)

Actually, script works right, but only when modem is as major device. It doesn't look for modems as subdevices of cards. For that, /proc/asound/cards won't show you any information, but aplay -l will. Therefore I added additional check in alsaload method in init.d script for situation when there is no "modem" string in /proc/asound/cards, but there is such string in aplay -l output. This check then takes row from output, extracts card and subdevice numbers, and create ALSA device address, e.t. "hw:0,6" and passes it to slmodemd as needed.

Patch is for debian/sl-modem-daemon.init file.

Updated patch, fixed one bug, which created wrong device for slmodemd to run. This works as it should.

Some comments:

 - Please don't call egrep and aplay -l twice. Do it once, save it in $output, and just use the variable.
 - I suspect that it also greps /proc/asound/cards further down. Please move that upwards and reuse the grep result.
 - echo $output -> echo "$output", to avoid automatic bash expansion
 - test "$line" -> [ -n "$line" ] for clarity and safety
 - readability: what should "if ! line=$() && line=$()" mean? Please don't mix tests and assignments with && and ||.

Can you please test it again with above changes and attach an updated patch? Once that still works, the change should also be submitted as a Debian bug.

Thank you!

Hi Martin,

There is requested new patch. Tried to implement all your suggestions, and in the way rewrote almost all alsaload. For me it now looks more compact and makes more sense. Tested it, works for both ALSA and slamr scenarios (can't test slmodemd modem:x scenario though). Bear with me, as it is my first patch for Bash script :)

Looking at the attachments in this bug report, I noticed that "sl-modem-daemon.init.patch" was not flagged as a patch. A patch contains changes to an Ubuntu package that will resolve a bug and this attachment is one! Subsequently, I've checked the patch flag for it. In the future when submitting patches please use the patch checkbox as there are some Launchpad searches that use this feature. Thanks for your contribution Peteris Krisjanis!

Peteris,

sorry, the patch doesn't apply at all against current Jaunty. Can you please attach your current, tested init script or rework the patch to apply against 2.9.11~20080817-3? Please also don't change the indentation (you used 3 spaces, original one uses 4).

The patch looks good now, thanks for improving it!

I'll forward this to Debian, too, unless you want to.

For third time, it shall be be perfect :)

Ok, here I attach patch which I got from diffing my current init.d script with 2.9.11-20080817-3. I also put right indentation (4 spaces, as original does).

However, I also attach my working init.d script in a case if there are again problem with this patch.

Uploaded and sent to Debian.

This bug was fixed in the package sl-modem - 2.9.11~20080817-3ubuntu1

---------------
sl-modem (2.9.11~20080817-3ubuntu1) jaunty; urgency=low

  * debian/sl-modem-daemon.init: Also check for modems which are
    subdevices, and thus only expose themselves in "aplay -l", not in
    /proc/asound/cards. Thanks to Peteris Krisjanis for the patch!
    (LP: #298424)

 -- Martin Pitt <email address hidden> Wed, 25 Feb 2009 12:11:03 +0100