Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for MAX44009 ambient light sensor #4907

Merged
merged 7 commits into from
Jan 24, 2019

Conversation

llagendijk
Copy link

Here is a driver for the MAX44009 ambient light sensor. This is a very low power device, that outputs the light level as a float. In order to support that I had to add a new HTTP_SNS_ILLUMINANCE_S as I needed to output a float (is this the right way?).
I have used xsns_91 for now pending allocation of an own driver number.
The size of the driver is 5k7. mainly caused by the calculations on line 57 in the MAX44009 driver. Is there a way to do float calculations that do not use so much code space?

Thanks for considering to include this in tasmota!
BR, Louis

Copy link
Contributor

@andrethomas andrethomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can re-use HTTP_SNS_ILLUMINANCE - you do not need to define a new HTTP_SNS_ILLUMINANCE_S

Copy link
Collaborator

@andrethomas2 andrethomas2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to not defining HTTP_SNS_ILLUMINANCE_S and re-using the existing HTTP_SNS_ILLUMINANCE please look at the unique values listed for default after reset/power-on and make your detection routine a little more robust using this to validate that you're actually finding a MAX44009 as opposed to just finding something on that particular I2C address.

image

@andrethomas2
Copy link
Collaborator

In addition, I don't think missed readings would apply because although the chip is configured in continuous measurement mode it will only provide whatever it has in its register on the 1-second interval you're reading it at... so maybe skim off a few bytes by using it without that - unless there's some technical or fundamental reason why you would want that logged output (which I can't imagine, but then I do not have the sensor so I am only reliant on what I see in the datasheet)

@ascillato
Copy link
Contributor

@llagendijk

Hi,

Any news on this?

@llagendijk
Copy link
Author

@llagendijk

Hi,

Any news on this?

Yeah, I am working on it: changes prove to be not so simple as I thought and I am a bit short on time. Please give me a few more days

- Added checks for improved MAX44009 detection
- removed HTTP_SNS_ILLUMINANCE_S (show only integer in web-interface)
- removed missed readings and repeated detection
@llagendijk
Copy link
Author

I made the requested changes:

  • Added checks for improved MAX44009 detection
  • removed HTTP_SNS_ILLUMINANCE_S (show only integer in web-interface)
  • removed missed readings and repeated detection
    Please comment if this solves your concerns

@andrethomas
Copy link
Contributor

andrethomas commented Jan 23, 2019

@llagendijk

Please check if this works for detection instead of reading all the registers - My intention was not to have you check all the registers... just one unique enough to validate a positive detect.

void Max4409Detect(void)
{
  if (max44009_found) { return; }

  uint16_t buffer;
  for (byte i = 0; 0 != max44009_addresses[i]; i++) {
    max44009_address = max44009_addresses[i];
    if (I2cValidRead16(&buffer, max44009_address, 0x05)) {
      if (0xFF00 == buffer) {
        max44009_found = 1;
        break;
      }
    }
  }
}

If the above works you can replace the 0x05 with a #define MAX4409_REG_THRESHOLD 0x05

If the above works then you can proceed using the same method to read 16 bit registers and simplify functions such as this one which means using existing code that is in support.ino therefore bringing down the final binary size especially in cases where other I2C sensors are also compiled into the code (which is usually the case)

bool Max4409Read_lum(void)
{
  uint16_t tmpdata;
  uint8_t regdata[2];

  max44009_valid = 0;
  
  if (I2cValidRead16(&tmpdata, max44009_address, REG_LUMINANCE)) {
    memcpy(&regdata,&tmpdata,sizeof(regdata));
    int exponent = (regdata[0] & 0xF0) >> 4;
    int mantissa = ((regdata[0] & 0x0F) << 4) | (regdata[1] & 0x0F);
    max44009_illuminance = (float)(((0x00000001 << exponent) * (float)mantissa) * 0.045);
    max44009_valid = 1;
    return true;
  }

  return false;
}

Another example of optimisation could be

    /* convert illuminance to string with suitable accuracy */
    uint8_t prec = 0;
    if (1000 > max44009_illuminance) { prec = 1; }
    if (100 > max44009_illuminance) { prec = 2; }
    if (10 > max44009_illuminance) { prec = 3; }
    dtostrf(max44009_illuminance, sizeof(illum_str) -1, prec, illum_str);

^^ shaves off 64 bytes of flash :)

If all the above works fine (remember I do not have a sensor so I have no way of testing these suggestions) you also don't need the Max4409Read_register function.

So with this in mind it brings your flash requirement down to 0.6k

- use functions from support.ino (had to split reading in Max4409Detect
  in 2 8 bits reads as the MAX44009 only supports 16 bits reads for
  luminance registers)
- Used the << instead of pow() to save a lot of xompiled code
- Improved float -> string conversion along the suggested lines
- Code size is now +/- 750 bytes (without other I2C sensors compiled in
  I think
@llagendijk
Copy link
Author

Changes implemented. In the detect function I had to split the 16 bit read into 2 8 bit reads as the MAX44009 supports 16bit reading only for luminance.
Left a bit of if() else if for the precision determination as I find that so much easier to read.

@arendst arendst merged commit 669f6c8 into arendst:development Jan 24, 2019
arendst added a commit that referenced this pull request Jan 24, 2019
Add support for MAX44009 Ambient Light sensor (#4907)
gemu2015 pushed a commit to gemu2015/Sonoff-Tasmota that referenced this pull request Jan 27, 2019
Add support for MAX44009 ambient light sensor
gemu2015 pushed a commit to gemu2015/Sonoff-Tasmota that referenced this pull request Jan 27, 2019
Add support for MAX44009 Ambient Light sensor (arendst#4907)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants