maximkulkin / esp-homekit

Apple HomeKit accessory server library for ESP-OPEN-RTOS
MIT License
1.1k stars 169 forks source link

Unexpected min/max value for homekit_format_int using HORIZONTAL_TILT_ANGLE #105

Closed buschco closed 4 years ago

buschco commented 4 years ago

Problem

On moving the tilt-angle slider in the home app the console prints this: !! HomeKit: [Client 4] Failed to update 1.14: value is not in range I inserted this line at the error position:

/* components/common/homekit/src/server.c line 2300 */
if (value < min_value || value > max_value) {
    printf("value: %d , rangeMin: %d, rangeMax: %d\n", value, ch->min_value, ch->max_value); // <- this is my insertion
    CLIENT_ERROR(context, "Failed to update %d.%d: value is not in range", aid, iid);
    return HAPStatus_InvalidValue;
}

and got this:

value: -8 , rangeMin: 1073646768, rangeMax: 1073646764
!!! HomeKit: [Client 4] Failed to update 1.14: value is not in range
>>> HomeKit: [Client 4] Update Characteristics
value: 15 , rangeMin: 1073646768, rangeMax: 1073646764
!!! HomeKit: [Client 4] Failed to update 1.14: value is not in range
>>> HomeKit: [Client 4] Update Characteristics
value: 25 , rangeMin: 1073646768, rangeMax: 1073646764
!!! HomeKit: [Client 4] Failed to update 1.14: value is not in range
>>> HomeKit: [Client 4] Update Characteristics
value: -21 , rangeMin: 1073646768, rangeMax: 1073646764
!!! HomeKit: [Client 4] Failed to update 1.14: value is not in range
>>> HomeKit: [Client 4] Update Characteristics

I think here is something wrong with the min/max values. Have I missed something here?

My Code

/* .... */
homekit_accessory_t *accessories[] = {
    HOMEKIT_ACCESSORY(.id=1, .category=homekit_accessory_category_window_covering, .services=(homekit_service_t*[]) {
        HOMEKIT_SERVICE(ACCESSORY_INFORMATION, .characteristics=(homekit_characteristic_t*[]) {
            /* NAME,MANUFACTURER,SERIAL_NUMBER,MODEL,FIRMWARE_REVISION */
            HOMEKIT_CHARACTERISTIC(IDENTIFY, window_covering_identify),
            NULL
        }),
        HOMEKIT_SERVICE(WINDOW_COVERING, .primary=true, .characteristics=(homekit_characteristic_t*[]) {
            HOMEKIT_CHARACTERISTIC(NAME, "Window blind"),
            &current_position,
            &target_position,
            &position_state,
            &current_tilt_angle,
            &target_tilt_angle,
            NULL
        }),
        NULL
    }),
    NULL
};

/* .... */

homekit_characteristic_t current_tilt_angle = {
    HOMEKIT_DECLARE_CHARACTERISTIC_CURRENT_HORIZONTAL_TILT_ANGLE(10)
};

homekit_characteristic_t target_tilt_angle = {
    HOMEKIT_DECLARE_CHARACTERISTIC_TARGET_HORIZONTAL_TILT_ANGLE(10)
};

/* .... */

Full code https://github.com/buschco/esp-homekit-demo/blob/tilt-angle/examples/window_covering/main.c

maximkulkin commented 4 years ago

Can you please fix your snippet and re-post output. In your printf statement, please change ch->min_value to min_value and same for max_value.

buschco commented 4 years ago

Thank you for the reply, your fix gave me more realistic values:

value: -29 , rangeMin: -90, rangeMax: -1
!!! HomeKit: [Client 4] Failed to update 1.14: value is not in range
>>> HomeKit: [Client 4] Update Characteristics
value: -3 , rangeMin: -90, rangeMax: -1
!!! HomeKit: [Client 4] Failed to update 1.14: value is not in range
>>> HomeKit: [Client 4] Update Characteristics
value: 29 , rangeMin: -90, rangeMax: -1
!!! HomeKit: [Client 4] Failed to update 1.14: value is not in range
>>> HomeKit: [Client 4] Update Characteristics
value: 47 , rangeMin: -90, rangeMax: -1
!!! HomeKit: [Client 4] Failed to update 1.14: value is not in range
>>> HomeKit: [Client 4] Update Characteristics

First I do not understand why max_value is -1 because in characteristics.h it says it should be 90 https://github.com/maximkulkin/esp-homekit/blob/master/include/homekit/characteristics.h#L1301

Secondly this line value: -29 , rangeMin: -90, rangeMax: -1 should be printed at all.

maximkulkin commented 4 years ago

Ok, I debugged your firmware and it looks there was some problem handling negative values. Since internally values are stored as int anyways (although that messes up support for uint64 values). Make sure you update to latest homekit version.

Also, you need to fix your firmware in terms of integer sizes and usage of signed/unsigned variables. Here is a patch I've got so far:

--- a/main.c    2019-09-17 19:27:30.000000000 -0700
+++ b/main.c    2019-09-17 17:35:26.000000000 -0700
@@ -105,34 +105,34 @@
     bool working = false;
     while (true) {
         int8_t direction = current_tilt_angle.value.int_value < target_tilt_angle.value.int_value ? 1 : -1;
-        uint8_t position = current_tilt_angle.value.int_value;
-        int16_t newTilt = position + direction;
+        int8_t position = current_tilt_angle.value.int_value;
+        int8_t newTilt = position + direction;

-        printf("tilt %u, target titl %u\n", newTilt, target_tilt_angle.value.int_value);
+        printf("tilt %d, target titl %d\n", (int)newTilt, target_tilt_angle.value.int_value);
         if(working == false && direction == 1) {
             gpio_write(gpio_up, on);
             working = true;
-            printf("call up with (%u)\n", target_tilt_angle.value.int_value);
+            printf("call up with (%d)\n", target_tilt_angle.value.int_value);
         } 
         if(working == false && direction == -1) {
             gpio_write(gpio_down, on);
             working = true;
-            printf("call down with (%u)\n", target_tilt_angle.value.int_value);
+            printf("call down with (%d)\n", target_tilt_angle.value.int_value);
         }
         current_tilt_angle.value.int_value = newTilt;
         homekit_characteristic_notify(&current_tilt_angle, current_tilt_angle.value);

         if (newTilt == target_tilt_angle.value.int_value) {
-            printf("reached destination %u\n", newTilt);
+            printf("reached destination %d\n", newTilt);
             if(direction == 1) {
                 gpio_write(gpio_up, off);
                 working = false;
-                printf("stop up with (%u)\n", target_tilt_angle.value.int_value);
+                printf("stop up with (%d)\n", target_tilt_angle.value.int_value);
             } 
             if(direction == -1) {
                 gpio_write(gpio_down, off);
                 working = false;
-                printf("stop down with (%u)\n", target_tilt_angle.value.int_value);
+                printf("stop down with (%d)\n", target_tilt_angle.value.int_value);
             }
             vTaskSuspend(updateTiltTask);
         }
buschco commented 4 years ago

Ok updating homekit fixed it 🤦‍♂ Thank you for your help and this awesome lib 👍