Add ALSA-based volume control and test logic#8
Add ALSA-based volume control and test logic#8WasabiFan wants to merge 7 commits intoev3dev:ev3dev-jessiefrom
Conversation
How about calling it "Sound" instead?
I would rather just have volume up and down (eventually, we should just have a graphic slider, but text is OK for now). If we make the change step by 10%, then you don't really need the half/min/max. Also, if the volume is set to 0, then behind the scenes, it should activate the mute if it exists, so we don't need a separate mute option.
It sounds like you are doing MVVM in which case I would call it IMixerElementViewModel. However, this is not how any of the other views/controllers are implemented. In other controllers, we bind properties of the data model to the view rather than binding properties of a view model in the view as you are doing here. There's nothing wrong with doing it the way you have here, but I would rather stay consistent with the existing patterns. By using the property binding feature of GObjects, we don't have to implement a function to attach to each notify signal. We only need a translate function for special cases like converting an enum to a string.
The alsa library is in pkg-config, so for now, you can add it to |
OK, I'll change that.
I think there needs to be either a mute option or a "minimum" option; having to hit the "volume down" button 10 times to mute the audio would probably get pretty annoying. How about I remove the mute button as it stands along with the half and max buttons and rename the "minimum" button to "Mute"? That way, you can still easily mute it, and it all fits on one page. Barring a slider, I was originally hoping for something simple that would let you use the left/right buttons to modify the volume so that you didn't need to select the separate menu items. The current capabilities of ev3devKit weren't enough to lay that out without significant additions, however. We'll have to revisit this sound UI if/when someone implements a slider or other similar control.
Yes, I aimed for something closer to MVVM or MVC because it seemed like there was a fair amount of manual UI management in the controller that I wanted to avoid. Essentially, I wanted to avoid the controller having any references to
Edit: After looking through the code, I've decided I won't try to add bindings to replace notify signals. There isn't anywhere meaningful that it would help simplify the code. The biggest issues are with interfacing with the control panel GUI, and that is limited by Gtk+'s capabilities, not our code.
Will-do.
Oh, I think I see what I did wrong -- I didn't clear the cache. After doing that, I was able to remove my other hack and it seems to be working. |
|
PR updated. |
src/AlsaBackedMixerElement.vala
Outdated
|
|
||
| namespace BrickManager { | ||
| public class AlsaBackedMixerElement: IMixerElementViewModel, Object { | ||
| private const SimpleChannelId primary_channel_id = SimpleChannelId.MONO; |
There was a problem hiding this comment.
Are you planning on changing this value? I don't see a need to. So, we don't really need a constant for it.
There was a problem hiding this comment.
It's the equivalent of a "magic number" and is used in two places, so having it as a constant makes the code more readable and maintainable. While I don't intend to change it, I already had to change it a few times in development, so there's no reason to think I wouldn't need to change it again.
This PR adds an "Audio" section in the main menu and associated sub-windows to allow volume control from brickman. The primary volume page looks like this:

There is also a list window that only appears if there are no mixer elements or there is more than one mixer element. That window can be accessed from the desktop test.
That menu has the following options:
This audio system is architected slightly differently from other current modules in brickman. I created a
ITestableMixerElementinterface (not a perfect name, but the best I could come up with) with two implementations: one of them is theAlsaBackedMixerElementand the other is theFakeMixerElement. The views operate onITestableMixerElements, so the controllers just instantiate whichever implementation they need and hand it to the view and the view handles the common display logic.The views connect to the
notifysignals on elements they are given, so they update automatically whenever the underlying elements change. This is mostly used by the test implementation, but it is also how the volume percentage on the main page operates: when the user hits a volume button, the view emits a signal; the controller then updates the volume value on the base element accordingly and the view gets the property change notification and updates the label.Some notes:
CMakeLists.txtadditions correctly. Specifically, on line 139, I addedasoundafter${DEPS_LIBRARIES}because I couldn't figure out where that variable was coming from. Guidance on where that should go would be appreciated.