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

[menu] Add optional allocator for ui menu classes #5

Merged
merged 2 commits into from
Jun 2, 2021

Conversation

MatthewMArnold
Copy link

This allows us to in-place allocate menus (whereas previously we were forced to use new). The main change I had to make was add an IAbstractView that isn't templated that is a parent of the AbstractView class to avoid circular template parameter dependencies. I'm not sure if there is a better way to do this.

@@ -30,12 +30,17 @@ namespace modm{
* \author Thorsten Lajewski
* \ingroup modm_ui_menu
*/
class AbstractMenu: public AbstractView
template<typename Allocator = allocator::Dynamic<IAbstractView> >
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be misunderstanding this but why do you need a template here? Is this sort of like using Allocator as an alias for allocator::Dynamic<IAbstractView>?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a templated class. So it allows us to specify the type of Allocator we want the class to use at compile time. The part Allocator = allocator::Dynamic<IAbstractView> is a default template parameter. If no template parameter is supplied the object will use allocator::Dynamic<IAbstractView> for the allocator.

src/modm/ui/menu/choice_menu.hpp Outdated Show resolved Hide resolved
@@ -112,4 +114,6 @@ namespace modm{
};
}

#include "choice_menu_impl.hpp"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this being included at the end of the file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this is a way of creating a template class with separate files for declaration/definition. Since you can't put template stuff in a .cpp file you put it in a _impl.hpp file and then include it at the bottom of the normal .hpp file. Then you can add a preprocessor statement at the top of the _impl.hpp file that makes it so your code won't compile if you try and include the _impl.hpp file in your own code (since you should be including the .hpp file instead).

src/modm/ui/menu/choice_menu_impl.hpp Show resolved Hide resolved
src/modm/ui/menu/choice_menu_impl.hpp Show resolved Hide resolved
src/modm/ui/menu/iabstract_view.hpp Outdated Show resolved Hide resolved
src/modm/ui/menu/iabstract_view.hpp Outdated Show resolved Hide resolved
{
modm::GraphicDisplay* display = &getViewStack()->getDisplay();
typename modm::GraphicDisplay* display = &this->getViewStack()->getDisplay();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused what typename is used here for.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gist is that since getViewStack returns a template object, we need to add typename before the type so the compiler knows this is actually a modm::GraphicDisplay.

Read a complete answer here: https://stackoverflow.com/questions/610245/where-and-why-do-i-have-to-put-the-template-and-typename-keywords

@MatthewMArnold MatthewMArnold force-pushed the viewstack-custom-allocator branch 2 times, most recently from 074aacc to 0056490 Compare May 2, 2021 17:28
This is because the gui stuff requires colored menus
but the menu code should not have to have colored
menus. This wouldn't be an issue if we could dynamic
cast but on certain devices you can't do this.
@MatthewMArnold MatthewMArnold merged commit 875b674 into develop Jun 2, 2021
MatthewMArnold added a commit that referenced this pull request Jul 20, 2021
* [menu] Add optional allocator for ui menu classes, used for destroying AbstractViews

* [menu] remove coupling between modm gui and modm menu view and menu

This is because the gui stuff requires colored menus
but the menu code should not have to have colored
menus. This wouldn't be an issue if we could dynamic
cast but on certain devices you can't do this.
manoliptram pushed a commit that referenced this pull request May 10, 2023
* [menu] Add optional allocator for ui menu classes, used for destroying AbstractViews

* [menu] remove coupling between modm gui and modm menu view and menu

This is because the gui stuff requires colored menus
but the menu code should not have to have colored
menus. This wouldn't be an issue if we could dynamic
cast but on certain devices you can't do this.
manoliptram pushed a commit that referenced this pull request May 10, 2023
* [menu] Add optional allocator for ui menu classes, used for destroying AbstractViews

* [menu] remove coupling between modm gui and modm menu view and menu

This is because the gui stuff requires colored menus
but the menu code should not have to have colored
menus. This wouldn't be an issue if we could dynamic
cast but on certain devices you can't do this.
manoliptram pushed a commit that referenced this pull request May 29, 2023
* [menu] Add optional allocator for ui menu classes, used for destroying AbstractViews

* [menu] remove coupling between modm gui and modm menu view and menu

This is because the gui stuff requires colored menus
but the menu code should not have to have colored
menus. This wouldn't be an issue if we could dynamic
cast but on certain devices you can't do this.
manoliptram pushed a commit that referenced this pull request May 29, 2023
* [menu] Add optional allocator for ui menu classes, used for destroying AbstractViews

* [menu] remove coupling between modm gui and modm menu view and menu

This is because the gui stuff requires colored menus
but the menu code should not have to have colored
menus. This wouldn't be an issue if we could dynamic
cast but on certain devices you can't do this.
manoliptram pushed a commit that referenced this pull request May 29, 2023
* [menu] Add optional allocator for ui menu classes, used for destroying AbstractViews

* [menu] remove coupling between modm gui and modm menu view and menu

This is because the gui stuff requires colored menus
but the menu code should not have to have colored
menus. This wouldn't be an issue if we could dynamic
cast but on certain devices you can't do this.
manoliptram pushed a commit that referenced this pull request May 29, 2023
* [menu] Add optional allocator for ui menu classes, used for destroying AbstractViews

* [menu] remove coupling between modm gui and modm menu view and menu

This is because the gui stuff requires colored menus
but the menu code should not have to have colored
menus. This wouldn't be an issue if we could dynamic
cast but on certain devices you can't do this.
nedonse pushed a commit that referenced this pull request Mar 18, 2024
* [menu] Add optional allocator for ui menu classes, used for destroying AbstractViews

* [menu] remove coupling between modm gui and modm menu view and menu

This is because the gui stuff requires colored menus
but the menu code should not have to have colored
menus. This wouldn't be an issue if we could dynamic
cast but on certain devices you can't do this.
nedonse pushed a commit that referenced this pull request Mar 18, 2024
* [menu] Add optional allocator for ui menu classes, used for destroying AbstractViews

* [menu] remove coupling between modm gui and modm menu view and menu

This is because the gui stuff requires colored menus
but the menu code should not have to have colored
menus. This wouldn't be an issue if we could dynamic
cast but on certain devices you can't do this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants