Skip to content

OLEDDisplay::setBrightness function unnecessary float calc and not according comments #422

@rob040

Description

@rob040

Describe the bug
The OLEDDisplay::setBrightness function unnecessary floating point calculation and is not according comments.

The current implementation:

void OLEDDisplay::setBrightness(uint8_t brightness) {
  uint8_t contrast = brightness;
  if (brightness < 128) {
    // Magic values to get a smooth/ step-free transition
    contrast = brightness * 1.171;
  } else {
    contrast = brightness * 1.171 - 43;
  }
  uint8_t precharge = 241;
  if (brightness == 0) {
    precharge = 0;
  }
  uint8_t comdetect = brightness / 8;

  setContrast(contrast, precharge, comdetect);
}
  1. There is an attempt to raise the contrast steeper than brightness, but breaks the (graph) line at 128 by lowering it with 43.
  2. That is definitively not a "smooth or step-free transition" because there is a big step backwards from 127 to 128.
  3. The use of floating point is completely unnecessary; if wanted, it could be solved with integer calculation.
  4. The contrast vs. brightness calculation is completely unnecessary; the 8% increase at midpoint (128), even if someone has calibrated his display and found that resulted in flat response, actually no one would and could notice that. Both OLED technology and human eye aren't that accurate.
  5. The precharge value (0xF1) is not according spec and description in setContrast():
    //0xF1 default, to lower the contrast, set to 1F
  6. The precharge value should vary from 0xF1 to 0x1F with contrast, and no nibble can be 0. Solution below.
  7. The comdetect = brightness / 8 can only reach value 0x1F, but spec and description in setContrast() says value 0x40 default and be lower with lower contrast.
  8. Note that this is ONLY for SSD1306 display; the SH1106 has its own setContrast() implementation, where precharge and comdetect are ignored. This is because when setting the precharge or comdetect will blank the display for 60-75ms.

Proposed, all integer, solution:

void OLEDDisplay::setBrightness(uint8_t brightness) {
  uint8_t contrast = brightness;
  uint8_t precharge = brightness/18+1;
  precharge = (precharge * 16) + (16-precharge);
  uint8_t comdetect = brightness / 4;
  setContrast(contrast, precharge, comdetect);
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions