ubi-agni / tactile_toolbox

software to handle tactile sensors in ROS
9 stars 5 forks source link

Range locking #22

Closed rhaschke closed 3 years ago

rhaschke commented 3 years ago

Replacement for #21: I implemented my suggestions of https://github.com/ubi-agni/tactile_toolbox/pull/21#pullrequestreview-508697633. @guihomework, please give it a try.

guihomework commented 3 years ago

After using this branch with @GereonBuescher today we (both me and him) noticed several things :

  1. Purple color for low saturation is okay, but it took some "large" amount below saturation to appear. I would say the smoothing is maybe a bit too "spread out". I remember having to put a value of 500 as a min with value reaching 0 to be able to see something (with a 4096 range overall).

  2. not knowing which value of the range was user selected was annoying. It would be cool to make the user changed/locked value bold.

  3. early Transition of Pinkish color did not convince use we where in "saturation" because it is too close to the red. I wonder if a saturation to "white" would work better there (like light red is probably different than orange or yellow so should still distinguish from other existing colors). Did not try it.

  1. Adding a button to reset/cancel the locking of all ranges in one go for all taxel group would be a nice feature, that we missed today.

  2. @GereonBuescher thought of maybe using a "texture" instead of a color change at saturation. Texture (for instance black thin stripes for low saturation and white stripes for high saturation) could be overlayed over the color (so keeping red/black as saturated value).

  3. in the #20 I asked about marking that the value is saturating on the displayed value but you argued that the displayed value was for the whole group (so kind of an average) and the value would become marked as saturated even if it is within the range. I agree. it won't work this way. I have a counter-proposition. What about indicating that one of the range (min or max) is being reached by "any" of the taxel value in the group. Like turning red when some values are about the range. This would help when the smoothing of the taxel display did not yet turn pink/purple to know we have values out of the locked range.

If you think those new ideas/features should be done in a new PR, I am fine with the state of this PR, it "works" but just not as ideal as I would like and lacks some practicality. I admit my PR did not provide so many features as I suggest here either, but this is now used by somebody else for the first time anyway and one discovered those needed while using.

rhaschke commented 3 years ago
  • Purple color for low saturation is okay, but it took some "large" amount below saturation to appear. I would say the smoothing is maybe a bit too "spread out".

Indeed the full saturation color is reached when over-/undershooting the given range by 50%.

  • Early transition of Pinkish color did not convince because it is too close to the red. I wonder if a saturation to "white" would work better there.

I can easily change this.

  • @GereonBuescher thought of maybe using a "texture" instead of a color change at saturation.

That's not supported by rviz.

  • Not knowing which value of the range was user selected was annoying. It would be cool to make the user changed/locked value bold.

I already implemented this. However, it's only possible to format the text field as a whole. So, it will not be possible to have different formats in the one-line summary. You need to open the fold and look for individual min and max values. Did you do that?

  • What about indicating that one of the range (min or max) is being reached by "any" of the taxel value in the group. Like turning red when some values are about the range. This would help when the smoothing of the taxel display did not yet turn pink/purple to know we have values out of the locked range.

That's not easily possible either. Again, the range property is only updated by the "accumulated" value, not yet considering the individual values.

  • Adding a button to reset/cancel the locking of all ranges in one go for all taxel group would be a nice feature, that we missed today.

The PropertyTreeView isn't really made to show a classical button... Need to check whether that's possible.

guihomework commented 3 years ago
  • Not knowing which value of the range was user selected was annoying. It would be cool to make the user changed/locked value bold.

I thought I already had something like this. Need to double check. However, it's only possible to format the text field as a whole.

I see. which is then maybe strange if the whole line is bold when only min OR max was edited.

So, it will not be possible to have different formats in the one-line summary. You need to open the fold and look for individual min and max values. Did you do that?

we did not open the fold of the range itself indeed.

  • What about indicating that one of the range (min or max) is being reached by "any" of the taxel value in the group. Like turning red when some values are about the range. This would help when the smoothing of the taxel display did not yet turn pink/purple to know we have values out of the locked range.

That's possible.

since it is not possible to change part of the font (it is logic that it is not easy, now that you say it, and how UI work) that would also change min and max color not telling which of the two was reached. Not ideal either.

  • Adding a button to reset/cancel the locking of all ranges in one go for all taxel group would be a nice feature, that we missed today.

The PropertyTreeView isn't really made to show a classical button... Need to check whether that's possible.

in the TF plugin, there is another entry "All frames" in addition to the frames. with a check box. acting on this one acts on all. What about creating a dummy group "all", including range and editing this one would change (or reset) all AND checking unchecking would check/ uncheck all. This looks to me like even better (I sometimes wanted to have ALL ranges to be 0 4096 of all of them to saturate at 100 or so, but did not have a nice idea you to implement it so did not request it, not it might be easy). This avoids the button at the same time.

We discovered those (features) needed while using.

Can you order them by priority of importance? Are they ordered already?

done by editing the original post.