Option to swap Z axis icons, adapt print status flow display for small screens

[ajs123]

As suggested in response to #57, an option to swap the Z axis icons on the fine tune and homing screens. Compilation with Z_PLUS_UPARROW defined will associate "Z+" with the up-arrow icon, etc. Of course, an entry in guppyscreen.json would be another (preferred?) approach.

Also, if GUPPY_SCREEN_SMALLSCREEN is defined, the flow display on the print status screen is made less crowded by dropping the precision to 1 decimal point.

[ballaswag]

I suggest to make this configurable as a toggle under the setting panel, e.g. Invert Z. The toggle can be similar to how the log setting works. Save an attribute to the guppyconfig.json on toggle and conditionally swap base on that value on start.

[ajs123]

A couple of questions:

(1) It looks like conf (of class Config) is where we always look for values of configurable items. All other copies of the option values are local, obtained via conf->get_json(...). We therefore trust the programmer :-) to ensure that json tags are interpreted consistently across units. Correct?

(2) In this block, I'm not seeing the need for the test if (loglevel < log_levels.size()). Perhaps a guard against error in consistency across source files?

    {
      auto idx = lv_dropdown_get_selected(loglevel_dd);
      if (idx != loglevel)
      {
        if (loglevel < log_levels.size())
        {
          loglevel = idx;
          auto ll = spdlog::level::from_str(log_levels[loglevel]);

          spdlog::set_level(ll);
          spdlog::flush_on(ll);
          spdlog::debug("setting log_level to {}", log_levels[loglevel]);
          conf->set<std::string>(conf->df() + "log_level", log_levels[loglevel]);
          conf->save();
        }
      }
    }

[ballaswag]

Yeah the whole json handling is kind of loose. At most I'm just checking for non null before access and not catching any type issues. So any misconfiguration in the config will most likely prevent guppy screen from starting. So we trust the user and the programmer for these loose assumptions 😀 .

I don't recall why I have the < check. Might be a useless check like you said.

[ajs123]

I'm seeing a problem with pulldown menus on the Nebula. Always get the last option regardless of where I tap. I imagine this has something to do with scaling. Maybe you'll see it with your knowledge of lvgl. Or if you tell me where to look I can try.

Would you prefer that I post as an issue?

[ballaswag]

Was this an issue before the calibration changes? I'm not sure if this is due to the new calibration changes or something related to the DPI setting that scales down the UI elements.

Can you try installing the version before the new calibration changes and see if the same issue exists?

[ajs123]

Pulldowns work with 0.21 with my screen scaling numbers.

[ballaswag]

Okay I see the issue, it's likely due to the lv_tc (calibration) that wraps the evdev_read(). However, I'm not entirely sure only the dropdown list is impacted. This is the only other similar issue I can find https://forum.lvgl.io/t/item-in-the-dropdown-list-is-not-selected/5381. But I don't understand the selected solution.

[ballaswag]

Okay, fixed the issue in the latest commit. The nightly should allow dropdown selections now.

[ajs123]

For me, with this change to lv_tc.c, choosing other than the last option on the list is working about 20% of the time. The screen blank pulldown seems to work slightly better with a finger which suggests something to do with spurious events.

When I was logging coordinates, without checking the state, I was definitely seeing numerous events for each touch, with some having clearly incorrect coordinates. I can't recall whether they were (0,0).

[ajs123]

I have this working on my Nebula pad. Here's what I was seeing and my solution:

Pulldown selections appear to be triggered upon release (makes sense). Logging selected points (raw/untransformed), here's an example of a failed selection:

Pull down the menu
1739, 2777 state 1
1734, 2777 state 1
1734, 3647 state 0

Tap on UP arrow - fail
1266, 1746 state 1
1260, 1749 state 1
4095, 3491 state 0

...and of a successful selection:

Pull down the menu
1449, 2694 state 1
1449, 2709 state 1
4095, 3705 state 0

Tap on UP arrow - suceed
1163, 1675 state 1
1163, 1685 state 1
1163, 1704 state 0

So the problem is that often, we're getting a bogus point of (xmax, ymax) as the only point reported upon release. I suppose that there is a sampling timing thing going on here, whether in the ststem device or elsewhere in the driver.

Here's my modified code. It ensures that we get one "release" point at the last-seen "pressed" point.

static lv_point_t last_pressed_point = {0,0};
static lv_indev_state_t last_state = 0;
lv_point_t _lv_tc_transform_point_indev(lv_indev_data_t *data) {    if(data->state == LV_INDEV_STATE_PRESSED) {
        // Pressed - just return coordinates
        last_pressed_point = data->point;
        last_state = data->state;
        fprintf(stderr,"Pressed at %d, %d\n", data->point.x, data->point.y);
        return lv_tc_transform_point(data->point);
    } else if(data->state == LV_INDEV_STATE_RELEASED && last_state == LV_INDEV_STATE_PRESSED) {
        // Released - ensure reporting of coordinates where the touch was last seen
        last_state = data->state;
        fprintf(stderr,"Released at %d, %d; return %d, %d\n", data->point.x, data->point.y, last_pressed_point.x, last_pressed_point.y);
        return lv_tc_transform_point(last_pressed_point);
    }
    // Invalid state
    lv_point_t point = {0, 0};
    return point;
}

With that code, here are two successful selections. Note the "correction" of the coordinates in the first case. The second is one that would have succeeded with the old code.

Pressed at 1454, 2746
Pressed at 1466, 2746
Released at 4095, 3695; return 1466, 2746
Pressed at 1369, 1751
Pressed at 1369, 1753
Released at 1369, 1740; return 1369, 1753

I can submit a PR, but I'm not sure how to do that while omitting my half-done Z axis icon changes.

[ballaswag]

@ajs123 does the nightly work for you with the dropdown selection. I was working consistently with calibrated touch for me when I tested it.

[ajs123]

The nighty from last night doesn't work for me. Might the Nebula be slower than your platform? This could be an asynchronous sampling thing. Even if event-driven, the system has to (somehow) catch the last touchpoint just before release.

[ballaswag]

I have this working on my Nebula pad. Here's what I was seeing and my solution:

Pulldown selections appear to be triggered upon release (makes sense). Logging selected points (raw/untransformed), here's an example of a failed selection:

Pull down the menu

1739, 2777 state 1

1734, 2777 state 1

1734, 3647 state 0

Tap on UP arrow - fail

1266, 1746 state 1

1260, 1749 state 1

4095, 3491 state 0

...and of a successful selection:

Pull down the menu

1449, 2694 state 1

1449, 2709 state 1

4095, 3705 state 0

Tap on UP arrow - suceed

1163, 1675 state 1

1163, 1685 state 1

1163, 1704 state 0

So the problem is that often, we're getting a bogus point of (xmax, ymax) as the only point reported upon release. I suppose that there is a sampling timing thing going on here, whether in the ststem device or elsewhere in the driver.

Here's my modified code. It ensures that we get one "release" point at the last-seen "pressed" point.

static lv_point_t last_pressed_point = {0,0};

static lv_indev_state_t last_state = 0;

lv_point_t _lv_tc_transform_point_indev(lv_indev_data_t *data) {    if(data->state == LV_INDEV_STATE_PRESSED) {

        // Pressed - just return coordinates

        last_pressed_point = data->point;

        last_state = data->state;

        fprintf(stderr,"Pressed at %d, %d\n", data->point.x, data->point.y);

        return lv_tc_transform_point(data->point);

    } else if(data->state == LV_INDEV_STATE_RELEASED && last_state == LV_INDEV_STATE_PRESSED) {

        // Released - ensure reporting of coordinates where the touch was last seen

        last_state = data->state;

        fprintf(stderr,"Released at %d, %d; return %d, %d\n", data->point.x, data->point.y, last_pressed_point.x, last_pressed_point.y);

        return lv_tc_transform_point(last_pressed_point);

    }

    // Invalid state

    lv_point_t point = {0, 0};

    return point;

}

With that code, here are two successful selections. Note the "correction" of the coordinates in the first case. The second is one that would have succeeded with the old code.

Pressed at 1454, 2746

Pressed at 1466, 2746

Released at 4095, 3695; return 1466, 2746

Pressed at 1369, 1751

Pressed at 1369, 1753

Released at 1369, 1740; return 1369, 1753

I can submit a PR, but I'm not sure how to do that while omitting my half-done Z axis icon changes.

Please create a PR for this so you get the credit for it. Looks good to me, but I'll run it on the 2 platform I have to make sure there's no other side effect and will merge it.

I certainly don't fully understand the touch flow. Th calibration is only is the transformation. Maybe it shouldn't even care about the state and just transform blindly (but that might be wasted compute because inactive points are continuously processed?). Not sure why the last touch gets missed.

[ajs123]

@ballaswag what do you think about the format of the Z+ = uparrow or downarrow option? It could be a pulldown (sensible with just two choices??), or a switch ("Z+ UP-arrow"). It could be an icon that flips but that's probably unnecessarily cute.

[ballaswag]

A toggle switch should suffice for now. Can probably add it below the log option and etc. settings.

[ajs123]

Done. It's al in this commit. I'm having trouble figuring out how to put just those in a PR.

[ballaswag]

Some minor comments. Looks good overall. Please clean up the dead code and formatting that's causing unnecessary diff.

Will give it a run later and go ahead with the merge.

[ajs123]

Cleanup done. Sorry for missing the code that was left from my use of a pulldown prior to going with a switch. Also, there was a block of code in sysinfo_panel.cpp that needed indentations to be fixed and unfortunately I used vscode auto-format which was set for the newline before the opening brace on if()s.

[ballaswag]

I merged and refactor some of your changes https://github.com/ballaswag/guppyscreen/pull/78. Your commit history is persisted.