Skip to content

Initial Library Release#1

Merged
zx100x100 merged 4 commits into
masterfrom
dev
Jun 11, 2026
Merged

Initial Library Release#1
zx100x100 merged 4 commits into
masterfrom
dev

Conversation

@zx100x100

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread .travis.yml Outdated
- pip install -U platformio

script:
- platformio ci --lib="." --board=uno --board=megaatmega2560 No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

travis is not used anymore, please move to github actions

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oops, wasn't expecting anyone else to find this so fast haha, mostly opened this PR so I could review the AI's work, working on that right now!

@zx100x100 zx100x100 marked this pull request as draft June 11, 2026 16:27
@zx100x100 zx100x100 marked this pull request as ready for review June 11, 2026 17:27
Comment thread TMP119.cpp
Comment on lines +41 to +57
void TMP119::read() {
// The TMP119 powers up in continuous-conversion mode, so the
// temperature register always holds the most recent conversion.
_wire->beginTransmission(_address);
_wire->write(TMP119_TEMP_REG);
_wire->endTransmission();

_wire->requestFrom(_address, (uint8_t)2);
int16_t raw = (_wire->read() << 8) | _wire->read();

// Each LSB represents 0.0078125 deg C, data is in 2's complement format
TEMP = raw * 0.0078125f;
}

float TMP119::temperature() {
return TEMP;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Over having the user to call read and temperature every time that he wants a temperature value, you can just create a single function called getTemperature.

tmp119.read();
const float temp = tmp119.temperature();
// vs
const float temp = tmp119.getTemperature();

The reason behind the get prefix is to follow the other functions of the class that already start with get/set.

This will also make the code simpler, since you can remove a function and also a private variable.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm thinking I might leave this one as-is. It's more consistent with the old library, and some other sensor libraries I've seen. I'm torn because the simplicity of one function is nice but I do like that currently tmp119.temperature(); doesn't require an I2C call and you can re-use it a bunch of times in place of needing to declare your own temperature variable.

Comment thread TMP119.h
Comment on lines +52 to +61
typedef enum {
TMP119_DELAY_0_MS,
TMP119_DELAY_125_MS,
TMP119_DELAY_250_MS,
TMP119_DELAY_500_MS,
TMP119_DELAY_1000_MS, // power-on default
TMP119_DELAY_4000_MS,
TMP119_DELAY_8000_MS,
TMP119_DELAY_16000_MS,
} tmp119_delay_t;

@patrickelectric patrickelectric Jun 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As the comment says.

/** Minimum standby delay between conversions in continuous-conversion mode
 *  (CONV[2:0], configuration register bits 9:7). This sets the standby time;
 *  the total conversion cycle time also depends on the averaging setting (see
 *  datasheet Table 8-6).

And in the datasheet:

Image

With the default register value of:

Image
0b00'100'01'0'0'0'0'0 = 220h
     │││ ││ │ │ │ │ └ ---
     │││ ││ │ │ │ └── Soft_Reset
     │││ ││ │ │ └──── DR/Alert
     │││ ││ │ └────── POL (ALERT pin polarity bit)
     │││ ││ └──────── T/nA (Therm/alert mode select)
     │││ ││ 
     │││ └┴────────── AVG (0b01: 8 Averaged conversions)
     └┴┴───────────── CONV (1s with AVG 01)

It's possible to validate the 1s as the default value indeed, but..
The current names of tmp119_delay_t are not correct. First, there is no configuration that matches the 0ms delay, and the standard is AVG 0b01, that results CONV[2:0] 0b000 as 125ms that's equal to CONV[2:0] 0b001.

There are two possible options IMO, one is to keep the current approach and having a different name for tmp119_delay_t values, maybe..

TMP119_DELAY_000
TMP119_DELAY_001
...

This is not clear for the user sadly.. but at least is clear the values, and the user can use it based in the table in the datasheet...

The second approach is better I believe, it requires as to read the AVG and set the delay based in desired time.

typedef enum {
    TMP119_DELAY_15_5_MS, // <-- Note this new name, that follows CONV 0b000 and AVG 0b00
    TMP119_DELAY_125_MS,
    TMP119_DELAY_250_MS,
    TMP119_DELAY_500_MS,
    TMP119_DELAY_1000_MS, // power-on default
    TMP119_DELAY_4000_MS,
    TMP119_DELAY_8000_MS,
    TMP119_DELAY_16000_MS,
} tmp119_delay_t;

...

bool TMP119::setReadDelay(tmp119_delay_t delay) {
    uint16_t config = getConfig();
    uint8_t avg = (config & TMP119_AVG_MASK) >> TMP119_AVG_SHIFT;
    switch(avg) {
        case 0b00:
            // Accept all delays
            break;
        case 0b01:
            if(delay < TMP119_DELAY_125_MS) {
                return false;
            }
            break;
        case 0b10:
            if(delay < TMP119_DELAY_500_MS) {
                return false;
            }
            break;
        case 0b11:
            if(delay < TMP119_DELAY_1000_MS) {
                return false;
            }
            break;
    };
    config = (config & ~TMP119_CONV_MASK) | ((uint16_t)delay << TMP119_CONV_SHIFT);
    return setConfig(config);
}

As you can see, the setReadDelay was modified to ensure that the correct parameter is being used and return false to invalid parameters.

Note: The code may be wrong, I just wrote it from the top of my head and looking at the datasheet.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is a great point, personally I prefer that the function directly sets the register (since this proposed function would cause issues if you try to set the delay before setting the averaging) but I've re-named some things and added a bunch more comments that make it more clear how this timing actually works without needing to read the datasheet.

low-noise readings.

-------------------------------
The MIT License (MIT)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since the license is already in the repository (LICENSE), it indicates that all the files in the same repository are already in the same license.
The license header can be added when different licenses are in play in the same repository and a license for a file does not follow the main repository license file (LICENSE).
If you really want to have a license header, I recommend using SPDX-License-Identifier.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sweet yeah I'll remove, old carry over from the TSYS01 library

@zx100x100 zx100x100 merged commit ade0abc into master Jun 11, 2026
2 checks passed
@zx100x100 zx100x100 deleted the dev branch June 11, 2026 21:18
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.

2 participants