-
-
Notifications
You must be signed in to change notification settings - Fork 80
Split HMI to functional variants #390
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
Conversation
This keeps the power button logic in one place. This used to be separate because the hmi logic was implicit in pbio/sys/bluetooth, but we don't have that anymore. By splitting this out of the sys/hmi module, we can make a platform specific HMI.
If we start a program on boot, the startup animation isn't finished. If we start the run animation then, we get a crash on running pbio_light_animation_stop_all after running the user program. Plenty of time wasted on this one. There is nothing wrong with the MicroPython exception handler. Thank you for reading.
Systems with an LCD will have their own UI, so we shouldn't be trying to share everything across all systems.
Relavant animations can be started when successfully starting a program, rather than having variant specific code in the main system loop.
We will be using the back button for normal poweroff, and losing your data is a confusing user experience. It can still be enabled with a config option for developers.
It's always been the center button, but we use the top left back button on EV3. It will be the dark gray button on NXT.
It is more intuitive to keep it on until the program starts or until you shut down.
Make it explicit that we have a proper restart.
There are three files named light_matrix.h, and several structures with a single field defined in one of the other ones. Some were directly including from the ../src/ directory. There were also funcs types, but they had only one function, and they always use a led_array. In practice, there will be only one light matrix. It has only one function to set one pixel on one led array. This simplifies it by cutting out some of the abstractions. And it still allows having more than one light matrix if we ever need it. We can remove the pbio/sys/light_matrix abstraction in the next commit.
This is used for light matrix tests, which now require this.
Separation of powers is nice, but since we have #include "../src/light/light_matrix.h" there is clearly something not quite right. Let's be pragmatic and make the APIs for animations public.
This lets us run it to completion before showing the UI, and keep variant-specific statuses like BLE_ADVERTISING in a dedicated deinit. Also drop the pbio/sys/light_matrix abstraction as explained in the prior commits. The HMI and user applications are now just users of the pbio/light_matrix device. For EV3, this means we can drop the depency of the light matrix. Instead, it will have its dedicated UI. To have something in place, we'll keep the grid-like display for now, but in hmi_lcd so we can easily replace it.
| #define PBSYS_CONFIG_FEATURE_PROGRAM_FORMAT_MULTI_MPY_V6_3_NATIVE (0) | ||
| #define PBSYS_CONFIG_BATTERY_TEMP_ESTIMATION (1) | ||
| #define PBSYS_CONFIG_HMI (1) | ||
| #define PBSYS_CONFIG_HMI_STOP_BUTTON (1 << 7) // top left, back button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not using macro names instead of hard-coding numbers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including pbio which includes pbdrv here seems to mess things up, leading them to be always zero.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would leave out the include here since this is just a config file and it would be up to the code that actually uses it to add the proper include (which it should have anyway if it is doing something with buttons).
Files that don't use this config option won't have a problem because it is just a #define so it won't try to resolve the symbol.
| } else { | ||
| while (last->next) { | ||
| last = last->next; | ||
| while (*pp) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a for loop. 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, like this?
pbio_os_process_t **pp;
for (pp = &process_list; *pp; pp = &(*pp)->next) {
if (*pp == process) {
// Already in the list.
return;
}
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup 😄
HMI
The Human Machine Interface (HMI) is split into functional variants. This way we can make a dedicated UI for platforms with an LCD and buttons such as the EV3 and NXT. They have a dedicated init and deinit which we can use for boot and shutdown animations, possibly including sound.
Poweroff
This generalizes the power down button so it doesn't have to be the center button. The EV3 will use the back button to stop programs. The button can be held for power down. We may additionally add an explicit shutdown option to the UI in the future.
Simplified light_matrix system API
There were 3 different light_matrix.h and light_matrix.c that were doing slightly different things. We really have only one light matrix. It is used by the HMI and by user programs. Simplifying this makes it much more transparent to build the HMI.
Disable EV3 hard reset by default
Since the back button is now used for normal poweroff, the hard reset is disabled by default. Developers can still turn it on if needed.