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

[Feature] XboxController button enums for easier mapping/history #1488

Closed
gyenesvi opened this issue Feb 26, 2024 · 14 comments
Closed

[Feature] XboxController button enums for easier mapping/history #1488

gyenesvi opened this issue Feb 26, 2024 · 14 comments
Labels
enhancement New feature or request topic: remote control Issues related to remotly controlling hubs

Comments

@gyenesvi
Copy link

Currently the XboxController buttons are identified by string names. Is there a reason why there are no enum values for this, just like Port and Direction? It would make it easier to find out what values exist and would be less error prone in coding, such as typing the string in lower case, mistyping, etc. Furthermore, a continuous range of enum values would make it easier to map buttons to certain properties such as previously pressed states and pressed times.

I have started using buttons for some actions, and quickly found that detecting events such as button down/up and long press will be inevitable. These can be derived from currently available pressed states with a history and additional data, such as down/up times. All these require mapping some data to buttons. Sure they can be done with dictionaries keyed with string button names, but that sounds a bit of a waste of memory in a constrained environment like a PU hub.

I was wondering if there are plans for native support for button events in the future, but found that it's something that can fairly easily be done in the Python layer, so no need to include them in the core FW, but having enums for them could make that simpler/more efficient.

@gyenesvi gyenesvi added enhancement New feature or request triage Issues that have not been triaged yet labels Feb 26, 2024
@laurensvalk
Copy link
Member

This is something I've been wondering about. Part of the reason is to keep the Parameter class consistent across all platforms, since not all of them have Xbox controller support, and the list might get even longer when we add more.

I don't know that performance would make that much a difference since MicroPython turns short strings into so called q-strings with an integer index, the same it would do for the enums. Although maybe it only does this combined with the const() method, I'm not actually sure.

Anyway, I was 50/50 about including them, so thanks for opening the issue... Now I have a better reason to include them in the enum after all 😄

@laurensvalk laurensvalk added topic: remote control Issues related to remotly controlling hubs and removed triage Issues that have not been triaged yet labels Feb 26, 2024
@gyenesvi
Copy link
Author

Do you mean you thought of including this in the Button class in pybricks.parameters? Or a separate class like XboxButton? First I was thinking that this is something that could get imported with the XboxController itself, but I see that's not possible, it needs to have its separate enum somewhere. Wonder whether pybricks.iodevices or pybricks.parameters would be a better place for it. Maybe placing it next to XboxController could resolve the consistency problem above.

As for performance, I was thinking that if the enum values would have a continuous range (starting from a small numer), then a (pre-allocated) flat array could be used for mapping values to them instead of a (dynamic) dictionary. But that may be too strict to expect.

@gyenesvi
Copy link
Author

By the way, is there a way to iterate through enum values in Pybricks? The usual Pythonic way of for d in Direction does not seem to work here, it says the type is not iterable.

@laurensvalk
Copy link
Member

Pending a few other open PRs, we'll have a fix for this one too. All buttons available through the Button enum 🙂

By the way, is there a way to iterate through enum values in Pybricks?

Not at the moment. Can you specify where it would be useful?

@gyenesvi
Copy link
Author

If putting all kinds of buttons to the same Button enum, can't that lead to name/value clashes at some point? I guess name clashes are possible to avoid with prefixes, though can get somewhat awkward, and I already see that different names have the same value, isn't that potentially error-prone (as in testing for one enum value that unintentionally matches another one as well)?

Also, this way it would be hard to enumerate all buttons of a specific device.

Can you specify where it would be useful?

For example, I was trying to define a mapping from all xbox controller buttons to unique ordinal numbers in continuous range for being able to associate state related variable to each button easily (stored in an array). Now what I can do is to enumerate all buttons manually, but that's also error prone if things change in the future. Simply being able to enumerate them would make it simpler/cleaner/less error prone.

@laurensvalk
Copy link
Member

They aren't really like C enums. Some internal numbers are only used to map them to internal button states.

For example, I was trying to define a mapping from all xbox controller buttons to unique ordinal numbers in continuous range for being able to associate state related variable to each button easily (stored in an array)

Dictionaries work well for this. You can just use the button as the index. Dictionary lookups are generally cheaper than an array if you include the overhead of keep track of the index yourself.

@laurensvalk
Copy link
Member

Xbox button enum entries have been added. You can try it (including the recent deadzone fix) from https://nightly.link/pybricks/pybricks-micropython/workflows/build/master when it finishes building in 15 minutes or so.

Thanks for opening this issue!

@gyenesvi
Copy link
Author

Dictionaries work well for this. You can just use the button as the index. Dictionary lookups are generally cheaper than an array if you include the overhead of keep track of the index yourself.

Sure, I can use dictionaries, but the idea was to avoid them, as they are in general more complex and less memory efficient than a plain array for a small size (dictionaries being dynamic data structures doing memory allocations all the time, as opposed to a fixed-size pre-allocated array). Furthermore, if the enum would keep track of the index (its value) itself, then that could also be eliminated, so additional data could be mapped to the buttons just with plain arrays.
But maybe that much of optimization is not required.. (just thought it might be because of all the consts and string interning being present)

Xbox button enum entries have been added. You can try it (including the recent deadzone fix) from https://nightly.link/pybricks/pybricks-micropython/workflows/build/master

Does that mean that if I install Pybricks again from beta.pybricks.com then I get that? Or does that require a manual build?

For testing, I was trying to install Pybricks on the Mindstorms hub, and saw that it requires to be plugged in. However, it's not clear if it needs to be plugged in for power, or if it requires a wired connection to the computer? Since on my current laptop there's only USB-C port, I cannot plug the USB-A cable of that hub into it, and only plugged into power, the hub was not visible for the install process. So I assume it requires wired data connection to the computer for install? If yes, I'll need to get a converter for that..
Also, is that wired connection required for downloading/updating the program later during development as well? Or hopefully that does work via bluetooth then?

@laurensvalk
Copy link
Member

During a normal firmware install, instead of choosing a hub, you can click on ‘advanced’, and choose your downloaded zip file. No need to build your own firmware.

USB is needed for firmware installation on the Spike and Mindstorms hubs, but after that they work with Bluetooth just like the Technic hub.

What you describe sounds essentially what dictionaries are for. Keeping your own arrays, indexes, and making your own lookups generally isn’t going to be much (if any) faster than just using the built in tools for it.

For any RC application of a mechanical toy with human input, computational differences in speed should be negligible.

@gyenesvi
Copy link
Author

During a normal firmware install, instead of choosing a hub, you can click on ‘advanced’, and choose your downloaded zip file. No need to build your own firmware.

Okay, that 's great, done this and tested. Both the button enums and the joystick deadzone works fine, thanks!

@laurensvalk
Copy link
Member

Thanks for testing, much appreciated.

@BertLindeman
Copy link

Both the button enums and the joystick deadzone works fine, thanks!

Victor, could you show us (me in fact) how to use the button enums?
Doc is not up to date yet (and I understand that!) but I need an example to get more into this.

Thanks,

Bert

@laurensvalk
Copy link
Member

Now, when you press the X button, XboxController.buttons.pressed() will include Button.X instead of the string “X”. Basically like the official remote, but with other buttons.

@laurensvalk
Copy link
Member

This is now also done for the blocks in the latest beta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request topic: remote control Issues related to remotly controlling hubs
Projects
None yet
Development

No branches or pull requests

3 participants