Skip to content

fix: Omit program slot ID when not specified. #2343

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

laurensvalk
Copy link
Member

We don't have a UI for it yet, so we should assume no slot is chosen rather than default to the first slot. This allows the user to select the slot on the hub without a front end UI. This is compatible with the existing protocol.

We don't have a UI for it, so we should assume no slot is chosen rather than default to the first slot.

This allows the user to select the slot on the hub without a front end UI.

This is compatible with the existing protocol.
@dlech
Copy link
Member

dlech commented May 19, 2025

I know this works with the current firmware, but looking forward, this might not work so well if we add additional parameters in the future. Could we change it so that instead of omitting the parameter that we have some sort of special value (e.g. (uint8_t)-1) that mean "current slot"?

@laurensvalk
Copy link
Member Author

That could work, although if we start adding other parameters (e.g filename or icon) it is perhaps time for a new command as part of however we might end up doing real file downloads.

I think we also need to keep the case without payload for backwards compatibility with older firmware (this was the intention, but I missed it when reviewing the Pybricks Code that added this).

@dlech
Copy link
Member

dlech commented May 19, 2025

I think we also need to keep the case without payload for backwards compatibility with older firmware

No, we already have sendLegacyStartUserProgramCommand for this.

@dlech
Copy link
Member

dlech commented May 19, 2025

And actually, we are already reading the selected slot from the status events, so we could just put that in as the slot number and not deal with any special case for "current slot".

@laurensvalk
Copy link
Member Author

OK. Happy to go either way. The special ID is a bit simpler but the latter might be a good exercise to start learning about redux and sagas...

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.

2 participants