#74 #55 fixing up the callback facilities before release.

Author: dave

src/tcMenu.cpp

@@ -249,6 +249,7 @@ void MenuManager::setupForEditing(MenuItem* item) {
         if(!notifyEditStarting(item)) return;
         auto* boolItem = (BooleanMenuItem*)item;
 		boolItem->setBoolean(!boolItem->getBoolean());
+		notifyEditEnd(item);
 	}
 	else if (ty == MENUTYPE_SCROLLER_VALUE) {
         if(!notifyEditStarting(item)) return;
@@ -305,15 +306,10 @@ MenuManager::MenuManager() : structureNotifier() {
     this->eepromRef = nullptr;
 }
 
-void MenuManager::addMenuAfter(MenuItem *existing, MenuItem* toAdd) {
-    if(existing->getNext()) {
-        toAdd->setNext(nullptr);
-        existing->setNext(toAdd);
-    }
-    else {
-        toAdd->setNext(existing->getNext());
-        existing->setNext(toAdd);
-    }
+void MenuManager::addMenuAfter(MenuItem *existing, MenuItem* toAdd, bool silent) {
+    toAdd->setNext(existing->getNext());
+    existing->setNext(toAdd);
+    if(!silent) notifyStructureChanged();
 }
 
 void MenuManager::addChangeNotification(MenuManagerObserver *observer) {

src/tcMenu.h

@@ -291,15 +291,16 @@ class MenuManager {
 
 	/**
 	 * Adds a menu item into the tree directly after the existing item provided. Never add an item that's already in
-	 * the tree. Don't forget to call menuStructureChanged() after you're done adding items
+	 * the tree. Don't forget that if you use silent, to call menuStructureChanged() after you're done adding items.
 	 * @param existing where in the tree the new item is to be added.
 	 * @param toAdd the item that should be added, must not be in the tree already.
+	 * @param silent do not run the structure changed callback.
 	 */
-	void addMenuAfter(MenuItem* existing, MenuItem* toAdd);
+	void addMenuAfter(MenuItem* existing, MenuItem* toAdd, bool silent = false);
 
 	/**
 	 * Call this method after making any structural change to the menu tree. For example adding a new menu item,
-	 * changing the item name or size parameters, modifying an info block, etc.
+	 * changing the item name or static data such as size parameters. For example: modifying an items name, etc.
 	 */
     void notifyStructureChanged();
 

tests/tcMenuCoreTests/menuManagerTests.cpp

@@ -1,5 +1,3 @@
-#ifndef MENU_MANAGER_TEST_H
-#define MENU_MANAGER_TEST_H
 
 #include <AUnit.h>
 #include <MockIoAbstraction.h>
@@ -71,4 +69,103 @@ test(saveAndLoadFromMenu) {
     assertFalse(eeprom.hasErrorOccurred());
 }
 
-#endif
+class TestMenuMgrObserver : public MenuManagerObserver {
+private:
+    bool structureChangedCalled{};
+    bool editStartedCalled{};
+    bool editEndedCalled{};
+    bool startReturnValue{};
+public:
+    TestMenuMgrObserver() = default;
+
+    void setStartReturn(bool toReturn) { startReturnValue = toReturn; }
+
+    void reset() {
+        structureChangedCalled = editStartedCalled = editEndedCalled = false;
+    }
+
+    void structureHasChanged() override {
+        structureChangedCalled = true;
+    }
+
+    bool menuEditStarting(MenuItem *item) override {
+        editStartedCalled = true;
+        return startReturnValue;
+    }
+
+    void menuEditEnded(MenuItem *item) override {
+        editEndedCalled = true;
+    }
+
+    bool didTriggerStructure() const { return structureChangedCalled; }
+    bool didTriggerStartEdit() const { return editStartedCalled; }
+    bool didTriggerEndEdit() const { return editEndedCalled; }
+} menuMgrObserver;
+
+const PROGMEM AnyMenuInfo testActionInfo = { "ActTest", 2394, 0xffff,  0, NO_CALLBACK };
+ActionMenuItem testActionItem(&testActionInfo, nullptr);
+
+const PROGMEM AnyMenuInfo testActionInfo2 = { "ActTest", 2394, 0xffff,  0, NO_CALLBACK };
+ActionMenuItem testActionItem2(&testActionInfo2, nullptr);
+
+test(addingItemsAndMenuCallbacks) {
+    menuMgr.setRootMenu(&textMenuItem1);
+    menuMgr.initWithoutInput(&noRenderer, &textMenuItem1);
+    menuMgr.addChangeNotification(&menuMgrObserver);
+
+    // first we add some menu items at the end of the menu and test the structure change call is made
+    menuMgr.addMenuAfter(&menuNumTwoDp, &testActionItem, true);
+    assertFalse(menuMgrObserver.didTriggerStructure());
+    menuMgr.addMenuAfter(&menuNumTwoDp, &testActionItem2);
+    assertTrue(menuMgrObserver.didTriggerStructure());
+
+    assertEqual(&testActionItem2, menuNumTwoDp.getNext());
+    assertEqual(&testActionItem, testActionItem2.getNext());
+    assertTrue(testActionItem.getNext() == nullptr);
+
+    // put back as it was before.
+    menuNumTwoDp.setNext(nullptr);
+
+    // now we test that when editing a bool, we get a commit callback.
+    menuMgrObserver.reset();
+    menuMgrObserver.setStartReturn(true);
+    auto currentMenuValue = boolItem1.getBoolean();
+    menuMgr.valueChanged(1); // boolItem1
+    assertTrue(boolItem1.isActive());
+    menuMgr.onMenuSelect(false);
+
+    assertTrue(menuMgrObserver.didTriggerEndEdit());
+    assertTrue(menuMgrObserver.didTriggerStartEdit());
+    assertTrue(boolItem1.getBoolean() != currentMenuValue);
+
+    // and now we test that when we return false in the start edit callback, we do not start editing.
+    menuMgrObserver.reset();
+    menuMgrObserver.setStartReturn(false);
+    menuMgr.onMenuSelect(false);
+    assertTrue(menuMgrObserver.didTriggerStartEdit());
+    assertFalse(menuMgrObserver.didTriggerEndEdit());
+    assertTrue(boolItem1.getBoolean() != currentMenuValue);
+
+    // now we move on to test an enum (integer) item and ensure we get the edit callbacks.
+    menuMgrObserver.reset();
+    menuMgrObserver.setStartReturn(true);
+    menuMgr.valueChanged(2); //  select menuEnum1
+    menuMgr.onMenuSelect(false); // start edit
+    assertTrue(menuMgrObserver.didTriggerStartEdit());
+    assertTrue(menuEnum1.isEditing());
+
+    menuMgr.valueChanged(1); // we are now editing, change the actual enum
+    menuMgr.onMenuSelect(false); // stop editing
+    assertTrue(menuMgrObserver.didTriggerEndEdit());
+    assertEqual((uint16_t)1, menuEnum1.getCurrentValue());
+
+    // lastly try an enum item that does not go into editing because the callback returned false.
+    menuMgrObserver.reset();
+    menuMgrObserver.setStartReturn(false);
+    menuMgr.valueChanged(2); // menuEnum1
+    menuMgr.onMenuSelect(false);
+    assertTrue(menuMgrObserver.didTriggerStartEdit());
+    assertFalse(menuEnum1.isEditing());
+
+}
+

tests/tcMenuCoreTests/tcMenuFixtures.h

@@ -2,6 +2,7 @@
 #define _TCMENU_FIXTURES_H_
 
 #include "tcUtil.h"
+#include "MenuItems.h"
 #include "RuntimeMenuItem.h"
 
 const PGM_TCM AnalogMenuInfo minfoNumTwoDp = { "TwoDpNum", 23, 32, 30000, NO_CALLBACK, 0, 100, "" };