Skip to content

A
adc-lib
Commit 1143b17d authored by Federico Vaga's avatar Federico Vaga
Browse files
Options

lib: bugfix do not override value on write 

The value that needs to be written must not be manipulated. However,
we do need to manipulate it in some case like, for example, when
configuring the offset. This is fixed by using a local variable.

The previous code was overriding the value with the manipulation
result: this is wrong and it led to the believe that this code was
working, actually it was masking another bug (fixed in another patch).

The following test in the integration test suite was passing due to
this bug, however it should have failed because the read-back value is
wrong.

    assert conf.value_get(PyAdcConf.ADC_CONF_CHN_OFFSET) == conf_rb.value_get(PyAdcConf.ADC_CONF_CHN_OFFSET)

The value in `conf` ended up to be the same as its read back `conf_rb`
due to the manilupation done direction on the pointer.

So the test needs to be fixed as well to check on this possible regression.
Signed-off-by: Federico Vaga's avatarFederico Vaga <federico.vaga@cern.ch>
Reported-by: 's avatarNicolas Magnin <nicolas.magnin@cern.ch>
parent af5a276f
Hide whitespace changes
Inline Side-by-side
Showing with 28 additions and 15 deletions
lib/fmc-adc-100m14b4cha.c
View file @ 1143b17d
......@@ -500,6 +500,7 @@ static int adc_100m14b4cha_config_chn(struct adc_dev *adc,
unsigned int direction)
{
struct __adc_dev_zio *fa = to_dev_zio(adc);
uint32_t value_tmp = *value;
char path[1024];
int err;
......@@ -520,12 +521,12 @@ static int adc_100m14b4cha_config_chn(struct adc_dev *adc,
case ADC_CONF_CHN_OFFSET:
snprintf(path, sizeof(path), "cset%u/ch%u-offset",
fa->cset, source);
*value = __offset_uv_to_raw(*value);
value_tmp = __offset_uv_to_raw(value_tmp);
break;
case __ADC_CONF_CHN_OFFSET_ZERO:
snprintf(path, sizeof(path), "cset%u/ch%u-offset-zero",
fa->cset, source);
*value = __offset_uv_to_raw(*value);
value_tmp = __offset_uv_to_raw(value_tmp);
break;
case ADC_CONF_CHN_SATURATION:
snprintf(path, sizeof(path), "cset%u/ch%u-saturation",
......@@ -535,7 +536,7 @@ static int adc_100m14b4cha_config_chn(struct adc_dev *adc,
errno = ADC_ENOCAP;
return -1;
}
err = adc_param[direction](adc, path, NULL, (int *)value);
err = adc_param[direction](adc, path, NULL, (int *)&value_tmp);
if (!err && direction == ADC_CONF_SET &&
index == ADC_CONF_CHN_RANGE) {
/*
......
pytest/fmc-adc-100m14b4cha/test_adc_getter_setter.py
View file @ 1143b17d
......@@ -64,7 +64,8 @@ class TestAdcGetterSetterChannel(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_CHN, channel)
conf_rb.mask_set(PyAdcConf.ADC_CONF_CHN_RANGE)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_CHN_RANGE) == conf_rb.value_get(PyAdcConf.ADC_CONF_CHN_RANGE)
assert vref == conf.value_get(PyAdcConf.ADC_CONF_CHN_RANGE)
assert vref == conf_rb.value_get(PyAdcConf.ADC_CONF_CHN_RANGE)
@pytest.mark.parametrize("termination", [True, False])
@pytest.mark.parametrize("channel", range(4))
......@@ -78,7 +79,8 @@ class TestAdcGetterSetterChannel(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_CHN, channel)
conf_rb.mask_set(PyAdcConf.ADC_CONF_CHN_TERMINATION)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_CHN_TERMINATION) == conf_rb.value_get(PyAdcConf.ADC_CONF_CHN_TERMINATION)
assert int(termination) == conf.value_get(PyAdcConf.ADC_CONF_CHN_TERMINATION)
assert int(termination) == conf_rb.value_get(PyAdcConf.ADC_CONF_CHN_TERMINATION)
@pytest.mark.parametrize("offset", range(-5000000, 4999999 + 1, 100000))
@pytest.mark.parametrize("channel", range(4))
......@@ -92,7 +94,8 @@ class TestAdcGetterSetterChannel(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_CHN, channel)
conf_rb.mask_set(PyAdcConf.ADC_CONF_CHN_OFFSET)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_CHN_OFFSET) == conf_rb.value_get(PyAdcConf.ADC_CONF_CHN_OFFSET)
assert offset == conf_rb.value_get(PyAdcConf.ADC_CONF_CHN_OFFSET)
assert offset == conf.value_get(PyAdcConf.ADC_CONF_CHN_OFFSET)
@pytest.mark.parametrize("offset", [-5000001, 5000000])
@pytest.mark.parametrize("channel", range(4))
......@@ -116,7 +119,8 @@ class TestAdcGetterSetterChannel(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_CHN, channel)
conf_rb.mask_set(PyAdcConf.ADC_CONF_CHN_SATURATION)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_CHN_SATURATION) == conf_rb.value_get(PyAdcConf.ADC_CONF_CHN_SATURATION)
assert saturation == conf.value_get(PyAdcConf.ADC_CONF_CHN_SATURATION)
assert saturation == conf_rb.value_get(PyAdcConf.ADC_CONF_CHN_SATURATION)
@pytest.mark.parametrize("saturation", [random.randrange(0x8000, 0xFFFFFFFF) for i in range(10)])
@pytest.mark.parametrize("channel", range(4))
......@@ -143,7 +147,8 @@ class TestAdcGetterSetterTriggerThreshold(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_TRG_THR, channel)
conf_rb.mask_set(PyAdcConf.ADC_CONF_TRG_THR_ENABLE)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_TRG_THR_ENABLE) == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_THR_ENABLE)
assert int(enable) == conf.value_get(PyAdcConf.ADC_CONF_TRG_THR_ENABLE)
assert int(enable) == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_THR_ENABLE)
@pytest.mark.parametrize("polarity", [0, 1])
@pytest.mark.parametrize("channel", range(4))
......@@ -157,7 +162,8 @@ class TestAdcGetterSetterTriggerThreshold(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_TRG_THR, channel)
conf_rb.mask_set(PyAdcConf.ADC_CONF_TRG_THR_POLARITY)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_TRG_THR_POLARITY) == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_THR_POLARITY)
assert polarity == conf.value_get(PyAdcConf.ADC_CONF_TRG_THR_POLARITY)
assert polarity == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_THR_POLARITY)
@pytest.mark.parametrize("delay", [2**x for x in range(32)])
@pytest.mark.parametrize("channel", range(4))
......@@ -171,7 +177,8 @@ class TestAdcGetterSetterTriggerThreshold(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_TRG_THR, channel)
conf_rb.mask_set(PyAdcConf.ADC_CONF_TRG_THR_DELAY)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_TRG_THR_DELAY) == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_THR_DELAY)
assert delay == conf.value_get(PyAdcConf.ADC_CONF_TRG_THR_DELAY)
assert delay == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_THR_DELAY)
@pytest.mark.parametrize("threshold", [2**x for x in range(15)])
@pytest.mark.parametrize("channel", range(4))
......@@ -185,7 +192,8 @@ class TestAdcGetterSetterTriggerThreshold(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_TRG_THR, channel)
conf_rb.mask_set(PyAdcConf.ADC_CONF_TRG_THR_THRESHOLD)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_TRG_THR_THRESHOLD) == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_THR_THRESHOLD)
assert threshold == conf.value_get(PyAdcConf.ADC_CONF_TRG_THR_THRESHOLD)
assert threshold == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_THR_THRESHOLD)
@pytest.mark.parametrize("threshold", [2**x for x in range(16, 32)])
@pytest.mark.parametrize("channel", range(4))
......@@ -213,7 +221,8 @@ class TestAdcGetterSetterTriggerThreshold(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_TRG_THR, channel)
conf_rb.mask_set(PyAdcConf.ADC_CONF_TRG_THR_HYSTERESIS)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_TRG_THR_HYSTERESIS) == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_THR_HYSTERESIS)
assert hysteresis == conf.value_get(PyAdcConf.ADC_CONF_TRG_THR_HYSTERESIS)
assert hysteresis == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_THR_HYSTERESIS)
@pytest.mark.parametrize("hysteresis", [2**x for x in range(16, 32)])
@pytest.mark.parametrize("channel", range(4))
......@@ -242,7 +251,8 @@ class TestAdcGetterSetterTriggerExternal(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_TRG_EXT, 0)
conf_rb.mask_set(PyAdcConf.ADC_CONF_TRG_EXT_ENABLE)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_TRG_EXT_ENABLE) == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_EXT_ENABLE)
assert int(enable) == conf.value_get(PyAdcConf.ADC_CONF_TRG_EXT_ENABLE)
assert int(enable) == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_EXT_ENABLE)
@pytest.mark.parametrize("polarity", [0, 1])
def test_adc_polarity(self, adc100m14b4cha, polarity):
......@@ -255,7 +265,8 @@ class TestAdcGetterSetterTriggerExternal(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_TRG_EXT, 0)
conf_rb.mask_set(PyAdcConf.ADC_CONF_TRG_EXT_POLARITY)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_TRG_EXT_POLARITY) == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_EXT_POLARITY)
assert polarity == conf.value_get(PyAdcConf.ADC_CONF_TRG_EXT_POLARITY)
assert polarity == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_EXT_POLARITY)
@pytest.mark.parametrize("delay", [2**x for x in range(32)])
def test_adc_delay(self, adc100m14b4cha, delay):
......@@ -268,4 +279,5 @@ class TestAdcGetterSetterTriggerExternal(object):
conf_rb = PyAdcConf(PyAdcConf.ADC_CONF_TYPE_TRG_EXT, 0)
conf_rb.mask_set(PyAdcConf.ADC_CONF_TRG_EXT_DELAY)
adc100m14b4cha.retrieve_config(conf_rb)
assert conf.value_get(PyAdcConf.ADC_CONF_TRG_EXT_DELAY) == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_EXT_DELAY)
assert delay == conf.value_get(PyAdcConf.ADC_CONF_TRG_EXT_DELAY)
assert delay == conf_rb.value_get(PyAdcConf.ADC_CONF_TRG_EXT_DELAY)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment