Incorrect documentation of mbl_mw_dataprocessor_multi_comparator_create() - Float handled as UInt8

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

  • 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)?

  • edited March 18

    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...

  • edited March 19

    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.

  • edited March 19

    What do you fire the comparator on?

  • edited March 19

    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.

  • edited March 22

    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.

  • So basically there is no bug in the APIs.

  • 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.

  • edited March 25

    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.