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

Adding support for hostname with dual keyboards for port #29

Merged

Conversation

matthawley
Copy link
Contributor

Fixed PR relating to after bad rebase, associated to PR #28 and bug #27

Copy link
Owner

@suchmememanyskill suchmememanyskill left a comment

Choose a reason for hiding this comment

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

Sorry that it took a bit to review.
I have some comments on this
I don't like:

  • That there's 2 keyboard instances, that either get awkwardly hidden or display over eachother.
  • That the keyboard is not automatically focused on the host textbox
  • That the OK button is in the bottom right

@suchmememanyskill
Copy link
Owner

diff --git a/CYD-Klipper/src/ui/ip_setup.cpp b/CYD-Klipper/src/ui/ip_setup.cpp
index 3be7bd7..b3e842b 100644
--- a/CYD-Klipper/src/ui/ip_setup.cpp
+++ b/CYD-Klipper/src/ui/ip_setup.cpp
@@ -13,6 +13,21 @@ lv_obj_t * hostEntry;
 lv_obj_t * portEntry;
 lv_obj_t * label = NULL;
 
+/* Create a custom keyboard to allow hostnames or ip addresses (a-z, 0 - 9, and -) */
+static const char * kb_map[] = {
+    "1", "2", "3", "4", "5", "6", "7", "8", "9", "0", LV_SYMBOL_BACKSPACE, "\n",
+    "q", "w", "e", "r", "t", "y", "u", "i", "o", "p", "\n",
+    "a", "s", "d", "f", "g", "h", "j", "k", "l", LV_SYMBOL_OK, "\n",
+    "z", "x", "c", "v", "b", "n", "m", ".", "-", " ", NULL
+};
+
+static const lv_btnmatrix_ctrl_t kb_ctrl[] = {
+    4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+    4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+    4, 4, 4, 4, 4, 4, 4, 4, 4, 6,
+    4, 4, 4, 4, 4, 4, 4, 4, 4, LV_BTNMATRIX_CTRL_HIDDEN | 4
+};
+
 void ip_init_inner();
 
 bool verify_ip(){
@@ -61,6 +76,18 @@ static void ta_event_cb(lv_event_t * e) {
             lv_label_set_text(label, "Failed to connect");
         }
     }
+    else {
+        return;
+    }
+
+    if (lv_obj_has_flag(ta, LV_OBJ_FLAG_USER_1))
+    {
+        lv_keyboard_set_mode(kb, LV_KEYBOARD_MODE_USER_1);
+    } 
+    else 
+    {
+        lv_keyboard_set_mode(kb, LV_KEYBOARD_MODE_NUMBER);
+    }
 }
 
 static void reset_btn_event_handler(lv_event_t * e){
@@ -150,6 +177,7 @@ void ip_init_inner(){
 
     hostEntry = lv_textarea_create(textbow_row);
     lv_textarea_set_one_line(hostEntry, true);
+    lv_obj_add_flag(hostEntry, LV_OBJ_FLAG_USER_1);
     lv_textarea_set_max_length(hostEntry, 63);
     lv_textarea_set_text(hostEntry, "");
     lv_obj_set_flex_grow(hostEntry, 3);
@@ -160,31 +188,12 @@ void ip_init_inner(){
     lv_textarea_set_text(portEntry, "80");
     lv_obj_set_flex_grow(portEntry, 1);
 
-	/* Create a custom keyboard to allow hostnames or ip addresses (a-z, 0 - 9, and -) */
-    static const char * kb_map[] = {
-        "1", "2", "3", "4", "5", "6", "7", "8", "9", "0", LV_SYMBOL_BACKSPACE, "\n",
-        "q", "w", "e", "r", "t", "y", "u", "i", "o", "p", "\n",
-        "a", "s", "d", "f", "g", "h", "j", "k", "l", "-", "\n",
-        " ", "z", "x", "c", "v", "b", "n", "m", ".", " ", LV_SYMBOL_OK, NULL
-    };
-
-    static const lv_btnmatrix_ctrl_t kb_ctrl[] = {
-        4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
-        4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
-        4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
-        LV_BTNMATRIX_CTRL_HIDDEN | 8, 4, 4, 4, 4, 4, 4, 4, 4, LV_BTNMATRIX_CTRL_HIDDEN | 2, 6
-    };
-    
-    lv_obj_t * hostKeyboard = lv_keyboard_create(root);
-    lv_keyboard_set_map(hostKeyboard, LV_KEYBOARD_MODE_USER_1, kb_map, kb_ctrl);
-    lv_keyboard_set_mode(hostKeyboard, LV_KEYBOARD_MODE_USER_1);
-    lv_obj_add_event_cb(hostEntry, ta_event_cb, LV_EVENT_ALL, hostKeyboard);
-    lv_obj_add_flag(hostKeyboard, LV_OBJ_FLAG_HIDDEN);
-
-    lv_obj_t * portKeyboard = lv_keyboard_create(root);
-    lv_keyboard_set_mode(portKeyboard, LV_KEYBOARD_MODE_NUMBER);
-    lv_obj_add_event_cb(portEntry, ta_event_cb, LV_EVENT_ALL, portKeyboard);
-    lv_obj_add_flag(portKeyboard, LV_OBJ_FLAG_HIDDEN);
+    lv_obj_t * keyboard = lv_keyboard_create(root);
+    lv_keyboard_set_map(keyboard, LV_KEYBOARD_MODE_USER_1, kb_map, kb_ctrl);
+    lv_obj_add_event_cb(portEntry, ta_event_cb, LV_EVENT_ALL, keyboard);
+    lv_obj_add_event_cb(hostEntry, ta_event_cb, LV_EVENT_ALL, keyboard);
+    lv_keyboard_set_mode(keyboard, LV_KEYBOARD_MODE_USER_1);
+    lv_keyboard_set_textarea(keyboard, hostEntry);
 }
 
 long last_data_update_ip = -10000;

These are my propsed changes to fix the above issues

@matthawley
Copy link
Contributor Author

@suchmememanyskill Updated PR and confirmed functional.

@suchmememanyskill
Copy link
Owner

Any reason why you reverted the proposed keyboard layout change?

@matthawley
Copy link
Contributor Author

Nope, just moved code myself and didn't recognize the change. Fixed.

@suchmememanyskill suchmememanyskill merged commit c18cd10 into suchmememanyskill:setup-ports Feb 6, 2024
@suchmememanyskill
Copy link
Owner

LGTM

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