Add auto-generated Bonsai interface #1
Conversation
device.yml
Outdated
| Start: 0 | ||
| StartAndStop: 1 | ||
| NoTrigger: 8 #TODO Is this correct? | ||
| TriggerAllConfig: #TODO UNCLEAR |
There was a problem hiding this comment.
Any time a trigger is received, all the channels are operated according to these groupmask options.
…evice.multipwm into bc-interface-autogen
bruno-f-cruz
requested review from
artursilva0,
glopesdev and
filcarv
and removed request for
artursilva0 and
glopesdev
Update .yml firmwareVersion
|
@glopesdev Can we merge this one? |
Fix reference to maskType in device schema
…lude-metadata Include device metadata file as embedded resource
|
@glopesdev Can we proceed with this merge? |
| TriggerAllModeConfig: | ||
| description: Available operation modes for "All" trigger input channel. | ||
| values: | ||
| Start: 0 | ||
| StartAndStop: 1 | ||
| Enable: 2 | ||
| EnableAndStop: 3 |
There was a problem hiding this comment.
Why is this enum different from TriggerModeConfig? It feels strange to have more options when configuring all channels at the same time versus one at a time, but maybe I am missing something here since I don't know the device.
There was a problem hiding this comment.
The additional options for TriggerModeConfig are not directly related to the Trigger Channels. The additional Enable options allow to enable the PWM channels and not the Trigger Channels. If I remember it was added here for convenience, since we could use this "All" channel to enable by an external signal all the PWM Channels.
There was a problem hiding this comment.
I see. In that case probably the name TriggerAllModeConfig is misleading since it provides no indication that different channel types other than trigger may also be configured. I can see two options to improve the situation:
- Keep the register name but remove the convenience functionality and make this register work only with the
TriggerModeConfiggroup mask (would require firmware changes); - Change the register name to encompass both trigger and PWM channels, e.g.
TriggerAndPwmModeorAllChannelMode. Also rename the group mask to match the register name with aConfigsuffix and rename the values in this group mask to include information about which channels are affected, e.g.TriggerStart,TriggerStartAndStop,PwmEnable,PwmEnableAndStop.
I would be happy with either choice, depending on how this register is meant to be used.
One last thing which is still not clear to me in this combined register is how the register can work with multiple channel types and be a group mask at the same time.
What happens if you write TriggerStart and then PwmEnable? Are these options mutually exclusive? Or will it leave the trigger mode at whatever value was set previously? In the latter case, what happens when you read the register?
Depending on the answers we could also consider the following options:
- Convert the register type into a bitmask;
- Make a separate register for all PWM channel config.
Both would also likely require firmware changes.
There was a problem hiding this comment.
I would prefer to improve the naming than making any changes to the firmware at this point. So I will modify the register, group masks name and values according to 2. and regenerate the interface if you have no additional suggestions.
These options are mutually exclusive.
This PR adds the first draft of the MultiPwm device metadata file, together with the corresponding auto-generated Bonsai high-level interface. The metadata YML file makes use of merge operators to avoid repeating declarations as much as possible, and currently relies on bitmask values to represent PWM and trigger input channels.