Split wifi hotspot config from pisound-btn package

Would it be ok to move the wifi hostpot related config files to a separate package ? At the moment
they are part of debian/ of pisound-btn, which makes removing it trickier when using Patchbox without a pisound card.

Yes, it was historically there since very early versions when we only had the pisound-btn package. :slight_smile:

You may change it as necessary on your system, we’ll note down to change this once we’ll be working on updating the OS image.

If you’d do a pull request for this, we could publish the new packages on our apt. :slight_smile:

Sure, happy to do a PR but I don’t know how to do one for a “new package” since it would be effectively a new repo wouldn’t it ? I can just publish it on my github and let you clone it. I’ll look into it, thanks

I think it would fit nicely into this repo:

Maybe it could be called patchbox-wifi?

As it’s a small package, it could be done similar to blokas-jack package, where the source itself is contained within the patchbox-os-debs repo.

So I’m working on this now (bear with me I’m new to Debian packaging…). I did hit a small snag:

In order for things not to break existing setup on apt-get update, what I would ideally need to do (based on instructions in PackageTransition - Debian Wiki case #8) is make the new version of pisound-btn depend on the “new” patchbox-wifi.

However, that introduces a dependency between pisound and patchbox which didn’t exist before. Ie, pisound-btn could previously be installed on its own without any patchbox related stuff.

Is that ok ?

Yes, that’s ok - pisound-btn package as well as the rest of our .deb packages are all hosted on our own apt.blokas.io server, so it’s fine for them to depend on each other for both Patchbox OS users and Raspberry Pi OS users (the apt server is set up as part of the install-pisound.sh script that users run to set it up)

Allright, found some time … gosh deb packages are … complicated. I’m mostly getting there. One issue though…

What to do with the scripts ? ie. enable/disable_wifi_hotspot.sh …

The .service file in the package refers to them, they are somewhat pisound specific at the moment as they get “git pulled” at postinst (which is … weird in itself, packages should be self contained).

I could/should move them into patchbox-wifi (and to a different location). This would probably work fine since patchbox-cli uses systemctl and doesn’t refer to them directly.

I’m thinking of creating a /usr/lib/patchbox/scripts.

The main issue is I can probably not move common.sh into it as we are getting back into rather pisound specific territory. That would mean taking out the LED stuff from those scripts.

Any better idea ?

One option could be to include common.sh if it exists, and have a fallback dummy flash_leds if not.

What’s your preference ?

So I’ve tentatively pushed my current work to:

GitHub - ozbenh/pisound: pisound kernel module and user-mode button branch patchbox-wifi
GitHub - ozbenh/patchbox-os-debs: Debian packages used in Patchbox OS image branch patchbox-wifi

For you to have a look and maybe test on an actual pisound system as I don’t have one before I do a PR (or if you want I’ll just do a PR and you can take it from there).

I probably need to polish the package descriptions/copyright/etc… still anyways.

The basic idea is we have two new packages:

  • patchbox-common has /usr/lib/patchbox/scripts/common.sh (and possibly more in the future). This is the “new” common.sh aimed at replacing the one in pisound. It will include a leds_common.sh provided by pisound if present otherwise provides stub LED functions. I haven’t yet changed the remaining scripts in pisound to use it, so the old one is still provided by pisound in the old location.

  • patchbox-wifi has all the wifi hotspot bits and pieces and some tweaks to take over the /etc/default/hostapd diversion from pisound-btn

Additionally pisound-btn is updated to 1.15, depends on patchbox-wifi and patchbox-wifi conflicts with pisound-btn < 1.15

Let me know what you think

1 Like

Sorry for not getting back earlier, we just had 2 public holidays here in Lithuania. :slight_smile: Thank you for providing the GitHub repos, I’ll try it out soon and get back to you.

Ah no worries, I hope you enjoyed the break !

I think it would make sense to keep the ‘common and LEDs’ logic entirely in the pisound-btn package, and don’t refer to it from the new patchbox-wifi package at all (including common.sh).

Instead, the enable_wifi_hotspot.sh and disable_wifi_hotspot.sh on pisound-btn side should do the LED logic part, and in the middle of that, just invoke the new enable_wifi_hotspot.sh and disable_wifi_hotspot.sh from the new patchbox-wifi package, to do all the WiFi hotspot logic.

There’s also toggle_wifi_hotspot.sh script on pisound-btn side which accesses systemd service directly - it’d be great to instead add a helper script in patchbox-wifi, called wifi_hotspot_status.sh or similar, to query the current state, and do the appropriate toggling logic.

It’s important to have both enable_wifi_hotspot.sh, disable_wifi_hotspot.sh in pisound-btn's script folder, as the scripts at /usr/local/pisound/scripts/pisound-btn are getting listed in pisound-config for remapping the button actions.

So in a nutshell, I’d propose dropping patchbox-common package, move out all the logic of WiFi .service and scripts to the new patchbox-wifi package, as you did already. Then change existing enable_wifi_hotspot.sh, disable_wifi_hotspot.sh, toggle_wifi_hotspot.sh scripts on pisound-btn to just keep the LED logic and invoke the patchbox-wifi scripts, abstract out any direct access to systemd calls on wifi-hotspot service with additional utility scripts.

I noticed enable hostspot restarts touchosc2midi, but disable hotspot doesn’t (so after disable hotspot, that service ends up failed).

I don’t know much about touchosc2midi but this seems … wrong.

What do you reckon should happen here ?

I’m not sure why you want to abstract from systemd … I find it rather cleaner to have the hostpot be a systemd service. That’s also how patchbox-cli uses it.

If I do what you asked, patchbox-cli will no longer toggle the LEDs, is that what you expect ? I’m fine with it personally :slight_smile: In fact I’ve started doing the changes, I just want to make sure we agree on the approach.

To clarify, with what I’m doing, patchbox-wifi provides scripts that are only meant to be used via systemd and systemd is the way to start/stop/query the hotspot. This is what “patchbox-cli” will do (and what other non-pisound things are doing).

So I don’t see how wifi_hotspot_status.sh would fit in that picture, unless it’s just a pisound-btn script, in which case it may as well be part of toggle_wifi_hotspot.sh

I’ve updated to two branches above in github with my changes. It includes a wifi_hotspot_status.sh in pisound’s scripts/pisound-btn that is used by toggle_wifi_hotspot.sh.

For the “abstraction” I decided to go for patchbox-cli … enable/disable just call the corresponding patchbox cli command. For status, I have that implemented but commented out, and use a shortcut
to systemd simply because loading patchbox cli for a simple status query is slow, and I don’t know
if you may want to “poll” that script in the future from pisound for other reasons, but feel free to
revert to the cleaner abstraction

You’re totally right! I was thinking about this a bit wrong before. :slight_smile:

Btw, in the latest repo state, the /usr/lib/patchbox-wifi/enable_wifi_hotspot.sh and disable scripts are missing in patchbox-wifi.

Anyway, you’re right about using systemd to control wifi-hotspot, and there’s no need to introduce dependencies then on patchbox-cli for Pisound, as the button scripts could just work by enabling, disabling and checking the status current status for toggle.

I think it’s fine for the LEDs to blink only when the wifi-hotspot state is being changed using the Pisound’s button.

Could you finalize this by making pisound-btn use systemd to control the WiFi, depend on patchbox-wifi package, and make sure the .sh files are commited to patchbox-wifi? :slight_smile:

Done. I’ve also done a PR on github !

Added some comments to Pisound’s PR