Fixed #4292 - Plotter scaling by adapting margin computation based on sign

[vicnevicne]

This is my proposed fix for #4292.
To reproduce the issue and check the fix, please use the following sketch and open Serial plotter. Cases 2 and 4 are correct only with the fix applied - see comparison in video: http://screencast.com/t/Wj0vxqjU

#define PAUSE_MS 5

void setup() {
  Serial.begin(9600);
}

void loop() {

  // 20 positive sawtooth : OK
  for (int n = 0; n < 20; n++) {
    for(int value = 0; value < 100; value++) {
      Serial.println(value);
      delay(PAUSE_MS);
    }
  }

  // 20 negative sawtooth : NOK
  for (int n = 0; n < 20; n++) {
    for(int value = 0; value < 100; value++) {
      Serial.println(-value);
      delay(PAUSE_MS);
    }
  }

  // 20 sawtooth high in positive range: OK
  for (int n = 0; n < 20; n++) {
    for(int value = 100; value < 200; value++) {
      Serial.println(value);
      delay(PAUSE_MS);
    }
  }

  // 20 sawtooth low in negative range: NOK
  for (int n = 0; n < 20; n++) {
    for(int value = 100; value < 200; value++) {
      Serial.println(-value);
      delay(PAUSE_MS);
    }
  }
}

[cmaglie]

Thank you, your proposal looks much better, BTW I see that there is always a lot of "unused" space in the graph.

IMHO a way to maximize the use of the whole graph area is to calulate the delta between max and min and "expand" max and min of a small percent of this delta, something like that:

double delta = maxY - minY; // This is the graph height

minY = minY - (delta / 10); // add a few more space under the graph
maxY = maxY + (delta / 10); // add a few more space over the graph

may you try with this? What I want to accomplish is to resize the graph always to the optimal size.

[vicnevicne]

Hi Cristian, and thanks for having a look.
The question of the "wasted space" did also worry me. In fact, the original code did not have any margins. Frederico added them to make it "look better" I guess.
I also thought the "factor 2" was huge but after a few tests, I think I understand the reason : the "Ticks" computation in the block that follows determines the Y-axis labels based on the minY and maxY. So by adding 10% margins, the labels do not match the actual min and max values, which looks weird.
For example, here is a screenshot of the sawtooth with your code :

The graph ranges from 100 to 200, which is not easy to figure out by looking at the Y labels 80, 160 and 240.
Probably that cosmetic issue would need to be adressed by passing "hints" to the Ticks computation logic or something, but I thought best to just fix here what clearly is a bug, while letting the percentage as a separate issue. But if the general feeling is that the 10% code is better, that's fine by me of course...

[vicnevicne]

Err...
In fact, I just wondered how it did look like before the addition of margins, and to my surprise, it behaves extremely well without any adjustment of minY and maxY, because the Ticks take care of leaving margins on top and bottom.
I thought maybe it was just my round numbers (0, 100, 200, ...) that made it look OK, but I quickly coded the generator below and the result looks really good to me:
http://www.screencast.com/t/ADbkB8ysvLm2

#define PAUSE_MS 2

void setup() {
  Serial.begin(9600);
}

void loop() {

  int a = random(-300, 300);
  int b = random(-300, 300);
  // 10  sawtooth
  for (int n = 0; n < 10; n++) {
    for(int step=0; step < 100; step++) {
      Serial.println(a + ((step * (b-a))/100.0));
      delay(PAUSE_MS);
    }
  }
}

Again, this is by basically undoing the computation change ffissore included in its commit 4cb72ce
but I see no drawback, so I would go for it.
All comments are welcome of course...

[cmaglie]

Looking at the commit message, it was added to solve this issue #3767 when the graph is a constant value that leads to minY equals maxY (probably there is a problem drawing the Y axis in this case). At this point, IMHO, the general solution is to adjust minY and maxY only if the delta (maxY - minY) is 0 or is less than a threshold, say 10, in that case we can just extend the range to reach 10, something like:

if ((maxY - minY) < 10) {
  double middle = (maxY + minY) / 2;
  maxY = middle + 5;
  minY = middle - 5;
}

may I ask you to check this last version?

[vicnevicne]

You are perfectly right. I thought the fix to #3767 he referred to was in the Tick class (part of the same commit) but the adjustment of minY and maxY is indeed required for the test case listed in that issue (plotting a constant value of 2).
Here are screenshots of that test case with no adjustment (top, not ok) and with your proposed solution (bottom, ok):


So that looks good to me :-)

[cmaglie]

Thanks @vicnevicne !