›

Incorrect documentation of mbl_mw_dataprocessor_multi_comparator_create() - Float handled as UInt8

Ben_RBen_R
edited March 2021 in Requests & Bug Reports

link to C++ documentation

Hey there, wanted to report what I believe to be an issue. I am using the most updated firmware on my MetaMotionR, using the swift API wrapper. Specifically, I use the .comparatorCreate command (calls mbl_mw_dataprocessor_multi_comparator_create() in C++)

I set up a comparator to trigger when a timer has fired 538 times with this code:

cycleCounter.comparatorCreate(op: MBL_MW_COMPARATOR_OP_EQ, mode: MBL_MW_COMPARATOR_MODE_ABSOLUTE, references: [Float(536)]).continueOnSuccessWith { monitor in

However, this comparator fires around 26 cycles, which happens to be: 536 - (255*2) = 26

Another comparator is set to fire after 596 cycles. In practice, it fires after 86 cycles: 596 - (255*2) = 86

In the C++ documentation for mbl_mw_dataprocessor_multi_comparator_create(),

METAWEAR_API int32_t mbl_mw_dataprocessor_multi_comparator_create (
MblMwDataSignal * source,
MblMwComparatorOperation op,
MblMwComparatorMode mode,
float references[],
uint8_t references_length,
void * context,
MblMwFnDataProcessor processor_created )

Setting the float: references[] value to anything above 255 loops the value back to 0.

I believe the Float inside this call is being treated as a UInt8, and not a float.

Comments

  • LauraLaura

    Two things.
    1. Post your code so I can make sure its correct.
    2. Look at the CPP code to check that its a Uint8 and not a Float (can you step through the code and check with a debugger)?

  • Ben_RBen_R
    edited March 2021

    Thanks Laura - really appreciate the time you give to everyone on this forum.

    (1) Here's a simplified version of the code, I've confirmed the problem still exists (granted, I reduced the loop period to 1000ms for expedition)

    device.timerCreate(period: 180000, immediateFire: false).continueOnSuccessWith() { lifecycle in
    lifecycle.counterCreateCount().continueOnSuccessWith() { cycleCounter in
        print("entered lifecycle counter subloop")
        //comparator reference max is treated like a UInt8, and not a float.
        cycleCounter.comparatorCreate(op: MBL_MW_COMPARATOR_OP_EQ, mode: MBL_MW_COMPARATOR_MODE_ABSOLUTE, references: [Float(536)]).continueOnSuccessWith { monitor in
            let ref: [Float] = [1.2]
            mbl_mw_event_record_commands(monitor)
            //flash LED during monitoring
            self.device.flashLED(color: .green, intensity: 1.0, _repeat: 255)
            monitor.eventEndRecord()
        }

        cycleCounter.comparatorCreate(op: MBL_MW_COMPARATOR_OP_GTE, mode: MBL_MW_COMPARATOR_MODE_ABSOLUTE, references: [Float(596)]).continueOnSuccessWith { safeFire in
            mbl_mw_event_record_commands(safeFire)
            //Red LED signal
            self.device.flashLED(color: .red, intensity: 1.0, _repeat: 1)
            safeFire.eventEndRecord()
        }
    }
}

    (2) This is my first time working with C++, so take the following with a grain of salt:

    My code enters switch case 1 of dataprocessor_config.cpp (line 238), when I'd expect it to enter case 4 instead. I suspect this may be the problem.

    I am guessing this code finds the lowest bit-size int type it can use for the given input, then assigns the value. The SCALE_MULTI_COMP_REFERENCE function (line 217, dataprocessor_config.cpp) is where I believe there is some faulty logic - the float is preserved through. Not sure what the issue could be here...

  • LauraLaura
    edited March 2021

    The task interface:

    /// Tasky interface to mbl_mw_dataprocessor_multi_comparator_create
/// Compare
public func comparatorCreate(op: MblMwComparatorOperation, mode: MblMwComparatorMode, references: [Float]) -> Task<OpaquePointer> {
    let source = TaskCompletionSource<OpaquePointer>()
    var references = references
    mbl_mw_dataprocessor_multi_comparator_create(self, op, mode, &references, UInt8(references.count), bridgeRetained(obj: source)) { (context, comparator) in
        let source: TaskCompletionSource<OpaquePointer> = bridgeTransfer(ptr: context!)
        if let comparator = comparator {
            source.trySet(result: comparator)
        } else {
            source.trySet(error: MetaWearError.operationFailed(message: "could not create comparator"))
        }
    }
    return source.task
}

    It clearly shows that references is [Float] type. Only the count is UInt8.

    I went further into the CPP lib:

    int32_t mbl_mw_dataprocessor_multi_comparator_create(MblMwDataSignal* source, MblMwComparatorOperation op, MblMwComparatorMode mode, float references[], 
    uint8_t references_length, void *context, MblMwFnDataProcessor processor_created) {
    return create_multi_comparator(source, op, mode, references, references_length, source->is_signed ? 1 : 0, context, processor_created);
}

    Again the reference is Float type.

  • LauraLaura
    edited March 2021

    What do you fire the comparator on?

  • LauraLaura
    edited March 2021

    Alright. Dug some more and ran your code (a version of anyways).
    Everything ends up typecasted to:

    struct MultiComparatorConfig {
        uint8_t is_signed:1;
        uint8_t length:2;
        uint8_t operation:3;
        uint8_t mode:2;
    uint8_t references[12];
};

    Which is unit8. You are right it's a mistake. You have to chain counters back to back in up to 255 increments to count higher numbers.
    I guess it must be a limitation of the chip in the old MetaWear boards and it's always stayed that way. I will see about changing the API call from float to uint8 in the next release.

  • LauraLaura
    edited March 2021

    Hey @Ben_R
    I had more time to look this weekend and actually it's not a mistake at all.
    If you see in the CPP lib:

    static int32_t create_multi_comparator(MblMwDataSignal* source, MblMwComparatorOperation op, MblMwComparatorMode mode, float references[], 
        uint8_t references_length, uint8_t is_signed, void *context, MblMwFnDataProcessor processor_created) {
    if (source->owner->firmware_revision < MULTI_COMPARE || source->length() > PROCESSOR_MAX_LENGTH) {
        return MBL_MW_STATUS_ERROR_UNSUPPORTED_PROCESSOR;
    }

    MultiComparatorConfig config;
    config.is_signed= is_signed;
    config.length= source->length() - 1;
    config.operation= op;
    config.mode= mode;

    uint8_t offset= 0;
    switch(source->length()) {
    case 1:
        SCALE_MULTI_COMP_REFERENCE(config, source, int8_t);
        break;
    case 2:
        SCALE_MULTI_COMP_REFERENCE(config, source, int16_t);
        break;
    case 4:
        SCALE_MULTI_COMP_REFERENCE(config, source, int32_t);
        break;
    }

    auto new_processor = MblMwDataProcessor::transform(source, type_to_id.at(DataProcessorType::COMPARATOR), &config);
    new_processor->config_size = 1 + offset;
    create_processor(source, new_processor, context, processor_created);

    return MBL_MW_STATUS_OK;
}

    The reference is type-casted based on the source size.

    You should never chain a timer with a comparator. You should chain a counter and a comparator instead.

  • LauraLaura

    So basically there is no bug in the APIs.

  • Ben_RBen_R

    Very interesting @Laura, thanks for the details. I currently have a timer that iterates a counter and checks a comparator. Would a comparator fire asynchronously, even if it is called outside of the timer? This is likely a fundamental misunderstanding on my part.

  • LauraLaura
    edited March 2021

    Yes, it can be used with a sensor data signal directly.
    PS: My bad for not seeing that earlier.

Sign In or Register to comment.