search

ubi-agni / tactile_toolbox

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

Add locked range property #21

Closed guihomework closed 3 years ago

guihomework commented 4 years ago

partially solves #20

rhaschke commented 4 years ago

Can you explain in which regard this only partially solves #20? What is missing?

guihomework commented 4 years ago

but it is also true that opening all sensors and checking a checkbox for each of them is cumbersome, so a secondary global checkbox enabling/disabling all the locking could be helpful too.

I did not implement a global lock range checkbox I did not know where/how to do it.

I also want to recall this issue was never raised because the solution was said to be imperfect during a meeting, and I cannot remember the argument against the solution of the PR. I have a vague memory that the argument against was related to the "place" where the checkbox was added. Maybe you wanted only a global checkbox, or maybe per group. I forgot.

So I decided to now raise the issue and provide the PR as is, if the argument was a serious argument, it will come back again while reviewing the PR and I am open to discuss any remark. This PR is just a quick hack I needed once, which could be turned in a feature.

guihomework commented 4 years ago

Looking into the existing code, I noticed that there is already a flag called manually_edited_. So, we could also use this flag to lock the range after an explicit user input. If any of the values is set to empty, we could reset the flag / unlock the range. What do you think of this approach?

What we need is a way to have the "locked" range saved within the rviz config. This was required by a colleague of ours, he wanted to show the extreme sensitivity of his sensor, every time one started rviz and loaded the special config with locked range one would then see the high sensitivity with a range locked on raw values [0; 20] while max of the sensor could go to 32768.

if the "manually_edited flag is saved, I would say the approach you proposed could work, but would in fact be a "hidden" feature (=one needs to know, whereas a check box with a label "lock-range" is intuitive)