Skip to content

Conversation

@orrpan
Copy link

@orrpan orrpan commented Apr 26, 2019

No description provided.

@kingosticks
Copy link
Member

Would you mind providing a bit of description in the commit messages as to what this solved.

@orrpan
Copy link
Author

orrpan commented Apr 26, 2019

If you want to set card to default, it is -1. Now there is a limit to only have positive integers.

class alsaaudio.Mixer(control='Master', id=0, cardindex=-1, device='default')

docs

@adamcik
Copy link
Member

adamcik commented Apr 26, 2019

Other option could be to leave min=0 and instead set optional=True and treat this field not being set / blank as -1 in the code (you might need to update the default config to be blank).

This IMO gives a nicer config exposed to end users than needing to know that -1 is a magical value.

Either way this is missing an update to the README documenting the new config semantics.

@orrpan
Copy link
Author

orrpan commented Apr 26, 2019

@adamcik thats a smart solution.
But when I test
schema['card'] = config.Integer(minimum=0, optional=True)
schema['card'] = config.Integer(minimum=-1, optional=True)

cardindex becomes 0, so it seems unnecessary to add further logic than having min=-1.
Yes, I have to update README.

@adamcik
Copy link
Member

adamcik commented Apr 26, 2019

https://github.com/mopidy/mopidy-alsamixer/blob/master/mopidy_alsamixer/ext.conf#L3 would need to blank, but I'm not 100% this would work. If it doesn't we can go ahead and merge what you have :-)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants