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

NRFX_COMP_DEFAULT_CONFIG uses wrong internal reference voltage #126

Open
LastLockJoe opened this issue Oct 4, 2024 · 1 comment
Open

Comments

@LastLockJoe
Copy link

LastLockJoe commented Oct 4, 2024

I was looking at the default configuration for the comparator and I noticed that the reference voltage in the comment does not match the reference voltage in the code.

/**
 * @brief COMP driver default configuration.
 *
 * This configuration sets up COMP with the following options:
 * - single-ended mode
 * - reference voltage: internal 1.8 V
 * - lower threshold: 0.5 V
 * - upper threshold: 1.5 V
 * - high speed mode
 * - hysteresis disabled
 * - current source disabled
 *
 * @param[in] _input Analog input.
 */
#define NRFX_COMP_DEFAULT_CONFIG(_input)                                           \
{                                                                                  \
    .reference          = NRF_COMP_REF_INT_1V2,                                    \
    .main_mode          = NRF_COMP_MAIN_MODE_SE,                                   \
    .threshold          = NRFX_COMP_CONFIG_TH,                                     \
    .speed_mode         = NRF_COMP_SP_MODE_HIGH,                                   \
    .hyst               = NRF_COMP_HYST_NO_HYST,                                   \
    NRFX_COND_CODE_1(NRF_COMP_HAS_ISOURCE, (.isource = NRF_COMP_ISOURCE_OFF,), ()) \
    .input              = (nrf_comp_input_t)_input,                                \
    .interrupt_priority = NRFX_COMP_DEFAULT_CONFIG_IRQ_PRIORITY                    \
}

Given the default threshold configuration uses 1.8, I assume the reference voltage should be changed to NRF_COMP_REF_INT_1V8.

/** @brief COMP threshold default configuration. */
#define NRFX_COMP_CONFIG_TH                                  \
{                                                            \
    .th_down = NRFX_COMP_VOLTAGE_THRESHOLD_TO_INT(0.5, 1.8), \
    .th_up   = NRFX_COMP_VOLTAGE_THRESHOLD_TO_INT(1.5, 1.8)  \
}

From my understanding of the comparator peripheral, setting the default internal ref to 1V8 would result in the comparator not working properly for boards with VDD = 1V8, ex: nRF7002 DK. Perhaps a warning should be added to note this?

@nika-nordic
Copy link
Contributor

hello @LastLockJoe , thanks for reaching out.

Looks like we have updated default configuration settings for the COMP driver but missed the doc alignment. 1.2 V reference voltage was chosen specifically as it is a common denominator across all nRF devices.

We will update the doc in next release

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

No branches or pull requests

2 participants