Experimental Physics and
Industrial Control System

Tonight's commit fixes the decimate filter and adds tests to ensure the field-logs get freed properly when dropped by the filter.

Work still to come on fixing the other filters and tests.


Diff comments:

> diff --git a/src/std/filters/decimate.c b/src/std/filters/decimate.c
> new file mode 100644
> index 0000000..d34884e
> --- /dev/null
> +++ b/src/std/filters/decimate.c
> @@ -0,0 +1,115 @@
> +/*************************************************************************\
> +* Copyright (c) 2019 UChicago Argonne LLC, as Operator of Argonne
> +*     National Laboratory.
> +* Copyright (c) 2010 Brookhaven National Laboratory.
> +* Copyright (c) 2010 Helmholtz-Zentrum Berlin
> +*     fuer Materialien und Energie GmbH.
> +* EPICS BASE is distributed subject to a Software License Agreement found
> +* in file LICENSE that is included with this distribution.
> +\*************************************************************************/
> +
> +/*
> + *  Authors: Ralph Lange <[email protected]>,
> + *           Andrew Johnson <[email protected]>
> + */
> +
> +#include <stdio.h>
> +
> +#include "freeList.h"
> +#include "db_field_log.h"
> +#include "chfPlugin.h"
> +#include "epicsExport.h"
> +
> +typedef struct myStruct {
> +    epicsInt32 n, i;
> +} myStruct;
> +
> +static void *myStructFreeList;
> +
> +static const
> +chfPluginArgDef opts[] = {
> +    chfInt32(myStruct, n, "n", 1, 0),
> +    chfPluginArgEnd
> +};
> +
> +static void * allocPvt(void)
> +{
> +    myStruct *my = (myStruct*) freeListCalloc(myStructFreeList);
> +    return (void *) my;
> +}
> +
> +static void freePvt(void *pvt)
> +{
> +    freeListFree(myStructFreeList, pvt);
> +}
> +
> +static int parse_ok(void *pvt)
> +{
> +    myStruct *my = (myStruct*) pvt;
> +
> +    if (my->n < 1)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +static db_field_log* filter(void* pvt, dbChannel *chan, db_field_log *pfl) {
> +    db_field_log *passfl = NULL;
> +    myStruct *my = (myStruct*) pvt;
> +    epicsInt32 i = my->i;
> +
> +    if (pfl->ctx == dbfl_context_read)
> +        return pfl;
> +
> +    if (i++ == 0)
> +        passfl = pfl;
> +
> +    if (i >= my->n)
> +        i = 0;
> +
> +    my->i = i;
> +    return passfl;

I agree this is required; I originally assumed I would have to do it, but the sync.c filter doesn't do that when in syncModeWhile or syncModeUnless so I assumed there was another mechanism in the chain processing that they relied on.

I will look at fixing sync.c as part of this MR, but aren't committing those changes just yet.

> +}
> +
> +static void channelRegisterPre(dbChannel *chan, void *pvt,
> +                               chPostEventFunc **cb_out, void **arg_out, db_field_log *probe)
> +{
> +    *cb_out = filter;
> +    *arg_out = pvt;
> +}
> +
> +static void channel_report(dbChannel *chan, void *pvt, int level, const unsigned short indent)
> +{
> +    myStruct *my = (myStruct*) pvt;
> +    printf("%*sDecimate (dec): n=%d, i=%d\n", indent, "",
> +           my->n, my->i);
> +}
> +
> +static chfPluginIf pif = {
> +    allocPvt,
> +    freePvt,
> +
> +    NULL, /* parse_error, */
> +    parse_ok,
> +
> +    NULL, /* channel_open, */
> +    channelRegisterPre,
> +    NULL, /* channelRegisterPost, */
> +    channel_report,
> +    NULL /* channel_close */
> +};
> +
> +static void decInitialize(void)
> +{
> +    static int firstTime = 1;
> +
> +    if (!firstTime) return;
> +    firstTime = 0;
> +
> +    if (!myStructFreeList)
> +        freeListInitPvt(&myStructFreeList, sizeof(myStruct), 64);
> +
> +    chfPluginRegister("dec", &pif, opts);
> +}
> +
> +epicsExportRegistrar(decInitialize);
> diff --git a/src/std/filters/test/decTest.c b/src/std/filters/test/decTest.c
> new file mode 100644
> index 0000000..cc9317e
> --- /dev/null
> +++ b/src/std/filters/test/decTest.c
> @@ -0,0 +1,273 @@
> +/*************************************************************************\
> +* Copyright (c) 2019 UChicago Argonne LLC, as Operator of Argonne
> +*     National Laboratory.
> +* Copyright (c) 2010 Brookhaven National Laboratory.
> +* Copyright (c) 2010 Helmholtz-Zentrum Berlin
> +*     fuer Materialien und Energie GmbH.
> +* EPICS BASE is distributed subject to a Software License Agreement found
> +* in file LICENSE that is included with this distribution.
> +\*************************************************************************/
> +
> +/*
> + *  Authors: Ralph Lange <[email protected]>,
> + *           Andrew Johnson <[email protected]>
> + */
> +
> +#include <string.h>
> +
> +#include "dbStaticLib.h"
> +#include "dbAccessDefs.h"
> +#include "db_field_log.h"
> +#include "dbCommon.h"
> +#include "dbChannel.h"
> +#include "registry.h"
> +#include "chfPlugin.h"
> +#include "errlog.h"
> +#include "dbmf.h"
> +#include "epicsUnitTest.h"
> +#include "dbUnitTest.h"
> +#include "epicsTime.h"
> +#include "testMain.h"
> +#include "osiFileName.h"
> +
> +void filterTest_registerRecordDeviceDriver(struct dbBase *);
> +
> +static int fl_equal(const db_field_log *pfl1, const db_field_log *pfl2) {
> +    return !(memcmp(pfl1, pfl2, sizeof(db_field_log)));
> +}
> +
> +static void fl_setup(dbChannel *chan, db_field_log *pfl, long val) {
> +    struct dbCommon  *prec = dbChannelRecord(chan);
> +
> +    pfl->ctx  = dbfl_context_event;
> +    pfl->type = dbfl_type_val;
> +    pfl->stat = prec->stat;
> +    pfl->sevr = prec->sevr;
> +    pfl->time = prec->time;
> +    pfl->field_type  = DBF_LONG;
> +    pfl->no_elements = 1;
> +    /*
> +     * use memcpy to avoid a bus error on
> +     * union copy of char in the db at an odd
> +     * address
> +     */
> +    memcpy(&pfl->u.v.field,
> +           dbChannelField(chan),
> +           dbChannelFieldSize(chan));
> +    pfl->u.v.field.dbf_long = val;
> +}
> +
> +static void testHead (char* title) {
> +    testDiag("--------------------------------------------------------");
> +    testDiag("%s", title);
> +    testDiag("--------------------------------------------------------");
> +}
> +
> +static void mustDrop(dbChannel *pch, db_field_log *pfl, char* m) {
> +    db_field_log *pfl2 = dbChannelRunPreChain(pch, pfl);
> +
> +    testOk(NULL == pfl2, "filter drops field_log (%s)", m);
> +}
> +
> +static void mustPass(dbChannel *pch, db_field_log *pfl, char* m) {
> +    db_field_log *pfl2 = dbChannelRunPreChain(pch, pfl);
> +
> +    testOk(pfl == pfl2, "filter passes field_log (%s)", m);
> +}
> +
> +static void checkAndOpenChannel(dbChannel *pch, const chFilterPlugin *plug) {
> +    ELLNODE *node;
> +    chFilter *filter;
> +    chPostEventFunc *cb_out = NULL;
> +    void *arg_out = NULL;
> +    db_field_log fl, fl1;
> +
> +    testDiag("Test filter structure and open channel");
> +
> +    testOk((ellCount(&pch->filters) == 1), "channel has one plugin");
> +
> +    fl_setup(pch, &fl, 1);
> +    fl1 = fl;
> +    node = ellFirst(&pch->filters);
> +    filter = CONTAINER(node, chFilter, list_node);
> +    plug->fif->channel_register_pre(filter, &cb_out, &arg_out, &fl1);
> +    testOk(cb_out && arg_out,
> +        "register_pre registers one filter with argument");
> +    testOk(fl_equal(&fl1, &fl),
> +        "register_pre does not change field_log data type");
> +
> +    testOk(!(dbChannelOpen(pch)), "dbChannel with plugin dec opened");
> +    node = ellFirst(&pch->pre_chain);
> +    filter = CONTAINER(node, chFilter, pre_node);
> +    testOk((ellCount(&pch->pre_chain) == 1 && filter->pre_arg != NULL),
> +        "dec has one filter with argument in pre chain");
> +    testOk((ellCount(&pch->post_chain) == 0),
> +        "sync has no filter in post chain");
> +}
> +
> +MAIN(decTest)
> +{
> +    dbChannel *pch;
> +    const chFilterPlugin *plug;
> +    char myname[] = "dec";
> +    db_field_log *pfl[10];
> +    int i;
> +    dbEventCtx evtctx;
> +
> +    testPlan(68);
> +
> +    testdbPrepare();
> +
> +    testdbReadDatabase("filterTest.dbd", NULL, NULL);
> +
> +    filterTest_registerRecordDeviceDriver(pdbbase);
> +
> +    testdbReadDatabase("xRecord.db", NULL, NULL);
> +
> +    eltc(0);
> +    testIocInitOk();
> +    eltc(1);
> +
> +    evtctx = db_init_events();
> +
> +    testOk(!!(plug = dbFindFilter(myname, strlen(myname))),
> +        "plugin '%s' registered correctly", myname);
> +
> +    /* N < 1 */
> +    testOk(!(pch = dbChannelCreate("x.VAL{\"dec\":{\"n\":-1}}")),
> +           "dbChannel with dec (n=-1) failed");
> +    testOk(!(pch = dbChannelCreate("x.VAL{\"dec\":{\"n\":0}}")),
> +           "dbChannel with dec (n=0) failed");
> +    /* Bad parms */
> +    testOk(!(pch = dbChannelCreate("x.VAL{\"dec\":{}}")),
> +           "dbChannel with dec (no parm) failed");
> +    testOk(!(pch = dbChannelCreate("x.VAL{\"dec\":{\"x\":true}}")),
> +           "dbChannel with dec (x=true) failed");
> +
> +    /* No Decimation (N=1) */
> +
> +    testHead("No Decimation (n=1)");
> +    testOk(!!(pch = dbChannelCreate("x.VAL{\"dec\":{\"n\":1}}")),
> +           "dbChannel with plugin dec (n=1) created");
> +
> +    checkAndOpenChannel(pch, plug);
> +
> +    for (i = 0; i < 5; i++) {
> +        pfl[i] = db_create_read_log(pch);
> +        fl_setup(pch, pfl[i], 10 + i);
> +    }
> +
> +    testDiag("Test event stream");
> +
> +    mustPass(pch, pfl[0], "i=0");
> +    mustPass(pch, pfl[1], "i=1");
> +    mustPass(pch, pfl[2], "i=2");
> +    mustPass(pch, pfl[3], "i=3");
> +    mustPass(pch, pfl[4], "i=4");
> +
> +    for (i = 0; i < 5; i++)
> +        db_delete_field_log(pfl[i]);
> +
> +    dbChannelDelete(pch);
> +
> +    /* Decimation (N=2) */
> +
> +    testHead("Decimation (n=2)");
> +    testOk(!!(pch = dbChannelCreate("x.VAL{\"dec\":{\"n\":2}}")),
> +           "dbChannel with plugin dec (n=2) created");
> +
> +    checkAndOpenChannel(pch, plug);
> +
> +    for (i = 0; i < 10; i++) {
> +        pfl[i] = db_create_read_log(pch);
> +        fl_setup(pch, pfl[i], 20 + i);
> +    }
> +
> +    testDiag("Test event stream");
> +
> +    mustPass(pch, pfl[0], "i=0");
> +    mustDrop(pch, pfl[1], "i=1");

As before this code was originally copied & adapted from syncTest.c. Looking at it more closely it seems to me now that if the filter drops a pfl and frees it as it's supposed to, the test code should *not* pass that pfl to db_delete_field_log() since it has already been freed once. If I'm right both dbndTest.c and syncTest.c need fixing.

I have added a routine to dbEvent.c that returns the number of items on the free list, so now test programs can check whether a filter has properly released a pfl when it's expected to. I added such checks to this decTest.c and removed the double-frees, and it now finishes with the expected number of available free items.

I'm not sure what other changes Michael is looking for in this code, will need more guidance on that.

I will work on fixing the other filters and filter tests.

> +    mustPass(pch, pfl[2], "i=2");
> +    mustDrop(pch, pfl[3], "i=3");
> +    mustPass(pch, pfl[4], "i=4");
> +    mustDrop(pch, pfl[5], "i=5");
> +    mustPass(pch, pfl[6], "i=6");
> +    mustDrop(pch, pfl[7], "i=7");
> +    mustPass(pch, pfl[8], "i=8");
> +    mustDrop(pch, pfl[9], "i=9");
> +
> +    for (i = 0; i < 10; i++)
> +        db_delete_field_log(pfl[i]);
> +
> +    dbChannelDelete(pch);
> +
> +    /* Decimation (N=3) */
> +
> +    testHead("Decimation (n=3)");
> +    testOk(!!(pch = dbChannelCreate("x.VAL{\"dec\":{\"n\":3}}")),
> +           "dbChannel with plugin dec (n=3) created");
> +
> +    checkAndOpenChannel(pch, plug);
> +
> +    for (i = 0; i < 10; i++) {
> +        pfl[i] = db_create_read_log(pch);
> +        fl_setup(pch, pfl[i], 30 + i);
> +    }
> +
> +    testDiag("Test event stream");
> +
> +    mustPass(pch, pfl[0], "i=0");
> +    mustDrop(pch, pfl[1], "i=1");
> +    mustDrop(pch, pfl[2], "i=2");
> +    mustPass(pch, pfl[3], "i=3");
> +    mustDrop(pch, pfl[4], "i=4");
> +    mustDrop(pch, pfl[5], "i=5");
> +    mustPass(pch, pfl[6], "i=6");
> +    mustDrop(pch, pfl[7], "i=7");
> +    mustDrop(pch, pfl[8], "i=8");
> +    mustPass(pch, pfl[9], "i=9");
> +
> +    for (i = 0; i < 10; i++)
> +        db_delete_field_log(pfl[i]);
> +
> +    dbChannelDelete(pch);
> +
> +    /* Decimation (N=4) */
> +
> +    testHead("Decimation (n=4)");
> +    testOk(!!(pch = dbChannelCreate("x.VAL{\"dec\":{\"n\":4}}")),
> +           "dbChannel with plugin dec (n=4) created");
> +
> +    checkAndOpenChannel(pch, plug);
> +
> +    for (i = 0; i < 10; i++) {
> +        pfl[i] = db_create_read_log(pch);
> +        fl_setup(pch, pfl[i], 40 + i);
> +    }
> +
> +    testDiag("Test event stream");
> +
> +    mustPass(pch, pfl[0], "i=0");
> +    mustDrop(pch, pfl[1], "i=1");
> +    mustDrop(pch, pfl[2], "i=2");
> +    mustDrop(pch, pfl[3], "i=3");
> +    mustPass(pch, pfl[4], "i=4");
> +    mustDrop(pch, pfl[5], "i=5");
> +    mustDrop(pch, pfl[6], "i=6");
> +    mustDrop(pch, pfl[7], "i=7");
> +    mustPass(pch, pfl[8], "i=8");
> +    mustDrop(pch, pfl[9], "i=9");
> +
> +    for (i = 0; i < 10; i++)
> +        db_delete_field_log(pfl[i]);
> +
> +    dbChannelDelete(pch);
> +
> +    db_close_events(evtctx);
> +
> +    testIocShutdownOk();
> +
> +    testdbCleanup();
> +
> +    return testDone();
> +}


-- 
https://code.launchpad.net/~anj/epics-base/+git/base-3.15/+merge/368347
Your team EPICS Core Developers is subscribed to branch epics-base:3.15.

References:

[Merge] ~anj/epics-base/+git/base-3.15:decimate-filter into epics-base:3.15 Andrew Johnson via Core-talk

Navigate by Date:

Prev: Build failed: epics-base base-integration-260 AppVeyor via Core-talk

Next: Build completed: epics-base base-7.0-261 AppVeyor via Core-talk

Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  <2019>  2020  2021  2022  2023  2024 

Navigate by Thread:

Prev: Re: [Merge] ~anj/epics-base/+git/base-3.15:decimate-filter into epics-base:3.15 Andrew Johnson via Core-talk

Next: Re: [Merge] ~anj/epics-base/+git/base-3.15:decimate-filter into epics-base:3.15 mdavidsaver via Core-talk

Index: 2002  2003  2004  2005  2006  2007  2008  2009  2010  2011  2012  2013  2014  2015  2016  2017  2018  <2019>  2020  2021  2022  2023  2024