897 enabledisable gripper from gripper page

[ProgramPhantom]

Think I've wired everything up correctly here.

First time touching the backend and I don't have much experience doing backend in general, so a thorough check would be appreciated! Lmk if anything needs changing.

[ProgramPhantom]

Does anyone know why this test failed? It appears to me that it's nothing to do with what I changed.

[1Blademaster]

Does anyone know why this test failed? It appears to me that it's nothing to do with what I changed.

Flaky copter test

[1Blademaster]

Added some comments. Can you also add some tests for the enabling and disabling of the gripper endpoints.

[ProgramPhantom]

Is ruff format . meant to be run in FGCS or FGCS/radio? Maybe its not accessing the right config file and causing files to be reformatted when they shouldn't be.

[1Blademaster]

Is ruff format . meant to be run in FGCS or FGCS/radio? Maybe its not accessing the right config file and causing files to be reformatted when they shouldn't be.

I think it's within the radio folder? Wherever the config file is

[ProgramPhantom]

All sorted I think! @1Blademaster

[1Blademaster]

All sorted I think! @1Blademaster

Cool. Will test on one of my drones when I get back 👍

[1Blademaster]

So on the FC where gripper does not exist, it tries to fetch the ENABLE param, fails, but then it tries to fetch the other params from cache? This is me going to config page, then clicking on gripper tab and that's all; not clicking on the enable gripper button.

INFO:fgcs:Changing state to config.gripper
ERROR:fgcs:Param GRIP_ENABLE not found in cached params
WARNING:fgcs:Gripper state could not be fetched from cache, fetching from drone
DEBUG:fgcs:Fetching gripper enabled state
DEBUG:fgcs:Timeout waiting for message PARAM_VALUE for controller params_13088
ERROR:fgcs:Did not receive GRIP_ENABLE within timeout
WARNING:fgcs:Gripper state could not be fetched from drone: Failed to get parameter GRIP_ENABLE, timed out
ERROR:fgcs:Param GRIP_CAN_ID not found in cached params
ERROR:fgcs:Param GRIP_AUTOCLOSE not found in cached params
ERROR:fgcs:Param GRIP_GRAB not found in cached params
ERROR:fgcs:Param GRIP_NEUTRAL not found in cached params
ERROR:fgcs:Param GRIP_REGRAB not found in cached params
ERROR:fgcs:Param GRIP_RELEASE not found in cached params
ERROR:fgcs:Param GRIP_TYPE not found in cached params

We don't need to search for those params, even from cache, if gripper enabled cannot be found right?

[ProgramPhantom]

Ah okay I see, it looks like replying on the gripper enabled parameter is not going to work for this. Do you know if MAVlink has a way of getting if the gripper exists at all? I'll need to add this to the controller so we can turn off these checks when the gripper is not found. Currently, it needs to check GRIPPER_ENABLED to decide whether to show the enable or disable button, but we really need a third option to say no gripper found.

[1Blademaster]

There's not any other way to check if gripper exists. We can still show the button even if gripper does note exist and let it timeout as it would.

[ProgramPhantom]

I don't see a way of avoiding the error of fetching GRIPPER_ENABLED then, there's no way to distinguish between a real erroneous interaction and the gripper not existing. GetSingleParam is setup to error if the parameter is not found. What should I do?

[1Blademaster]

We can't avoid gripper enable; that's fine. But we can avoid the rest of the params, even from the cache (shown in log in earlier comment).

[ProgramPhantom]

I think I've found the source of what you're encountering, lmk if this fixes it.

[1Blademaster]

You'll need to add getGripperEnabled to the dependency list of the use effect.

[ProgramPhantom]

Oh okay - how come?

[1Blademaster]

Any variables used within a use effect should be in the dependency array, e.g. if getGripperEnabled get's updated then the use effect won't re-execute.

[ProgramPhantom]

Ah I see the confusion - I have added middleware so that when the "set_gripper_enabled_result" is emitted, emitGetGripperConfig() is automatically run, so I didn't expect that the dispatch in the component would have to be run again. One of these is probably unnecessary then - lmk if strange things are still happening.

Here's the middleware which I added to do this.

  socket.socket.on(
    ConfigSpecificSocketEvents.onSetGripperEnabledResult,
    (result) => {
      if (result.success) {
        store.dispatch(setGetGripperEnabled(true))
        store.dispatch(emitGetGripperConfig())
        showSuccessNotification(result.message)
      } else {
        showErrorNotification(result.message)
      }
    },
  )

  socket.socket.on(
    ConfigSpecificSocketEvents.onSetGripperDisabledResult,
    (result) => {
      if (result.success) {
        store.dispatch(setGetGripperEnabled(false))
        store.dispatch(emitGetGripperConfig())
        showSuccessNotification(result.message)
      } else {
        showErrorNotification(result.message)
      }
    },
  )

[1Blademaster]

Almost there; if I enable gripper through params then go back to config page; it doesn't update/fetch the gripper enabled param?

[ProgramPhantom]

I can certainly remove the emitGetGripperConfig() on disabled, lmk what's happening with that last commit and then I'll do that.

[1Blademaster]

I can certainly remove the emitGetGripperConfig() on disabled, lmk what's happening with that last commit and then I'll do that.

Yeah probably good idea.

[ProgramPhantom]

Almost there; if I enable gripper through params then go back to config page; it doesn't update/fetch the gripper enabled param?

Hmm it should do, the change in UI is controlled by the store, which I assume is working correctly on the params page. I'll have a look.

[ProgramPhantom]

Hmm, I've got a solution but I'm not sure if you'll be happy.
The problem is the setting of parameters does not actually update the redux store, in particular getGripperEnabled, so it doesn't change on the frontend. I don't think it's possible to precisely select this change when setting params in order to call emitSetGripperDisabled etc. Therefore the only way I can see this working is to call emitGetGripperEnabled everytime the gripper tab is opened, so it can check if that parameter is changed. This does result in the very brief loading screen when navigating the the gripper tab, but seems to work. Is this suitable?

[ProgramPhantom]

I've done some rooting around and found this:

updateParamValue: (state, action) => {
 state.params = state.params.map((item) =>
    item.param_id === action.payload.param_id
      ? { ...item, param_value: action.payload.param_value }
      : item,
  )
},

which looks like it does what I was talking about in the prior message. This does indeed set GRIP_ENABLE, but this piece of state is actually separate to the state controlling the gripper on the config page. The config slice has it's own separate getGripperEnabled state, which is what I have been working with and what the UI responds to. This is creating quite a confusing duplication of the source of truth, and is the reason why changing the parameter on the parameters page does not change the UI - getGripperEnabled doesn't update.

What is the best solution here? The above solution works. I don't see any other way to easily make this an automatic thing, the config state simply needs to constantly observe the params slice in order to keep these things in sync.

[1Blademaster]

Yeah I was thinking that we need to probably refactor the config pages so they align with the params page better (in terms of params etc). For now though, having a small loading screen is fine; it's what we do on other pages atm as well. We'll make it automatic in another PR with the params link.

[ProgramPhantom]

Okay, sounds good. Write params now updates the gripper enabled state. Changing the gripper enabled state on the gripper page does not change the parameter value on the params page however, without a manual "refresh params". Is this normal?

[1Blademaster]

Yes that's normal.. for now. Will fix once we refactor.

[ProgramPhantom]

This was working on mine - hopefully on yours too

[1Blademaster]

Got there finally, seems to be working for me on my two FCs.

[ProgramPhantom]

Finally 🤣