project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.23k stars 1.92k forks source link

[Platform] Collision of macro function "ArraySize" in CodeUtils.h from #5646 #30140

Open bmryu0501 opened 8 months ago

bmryu0501 commented 8 months ago

Reproduction steps

Hello,

I am trying to implement On-Device-AI on an esp32 matter device using TensorFlow Lite for Microcontrollers. However, there's a naming collision between the ArraySize macro function defined in connectedhomeip's CodeUtils.h and the ArraySize function in tflite-micro. They are declared at the following paths:

connectedhomeip/src/lib/support/CodeUtils.h connectedhomeip/third_party/silabs/gecko_sdk/util/third_party/tflite-micro/tensorflow/lite/kernels/internal/types.h What would be the best solution to resolve this situation?

Here are the solutions I've considered:

  1. Enclose ArraySize in CodeUtils.h within a namespace. I believe ArraySize in CodeUtils.h was designed to be compatible with C as well. I'm unsure whether using a namespace would defeat that purpose. Additionally, I'm uncertain about how to address this with the existing codebase.

  2. Rename ArraySize in types.h. As a temporary measure, I could rename the ArraySize in tflite. Given that there are relatively few references to ArraySize in tflite, it might be time-consuming but feasible to change the name. (However, I'm not fond of this approach.)

  3. Configure through settings. I thought someone might have considered this and provided a configuration setting, but I couldn't find any.

Considering future scenarios, I think it would be best to address the ArraySize macro function in CodeUtils.h. However, from a learner's perspective, I'm unsure which is the right approach.

Please help.

This is an error message when I've run idf.py build

In file included from /home/ssafy/esp/esp-matter/examples/dimmable_tflite/managed_components/espressif__esp-tflite-micro/tensorflow/lite/micro/kernels/conv.h:22,
                 from /home/ssafy/esp/esp-matter/examples/dimmable_tflite/managed_components/espressif__esp-tflite-micro/tensorflow/lite/micro/micro_mutable_op_resolver.h:27,
                 from /home/ssafy/esp/esp-matter/examples/dimmable_tflite/main/app_main.cpp:23:
/home/ssafy/esp/esp-matter/examples/dimmable_tflite/managed_components/espressif__esp-tflite-micro/tensorflow/lite/kernels/internal/types.h:243:46: error: macro "ArraySize" passed 2 arguments, but takes just 1
  243 | int ArraySize(const Dims<N>& array, int index) {
      |                                              ^
In file included from /home/ssafy/esp/esp-matter/connectedhomeip/connectedhomeip/src/system/SystemLayer.h:33,
                 from /home/ssafy/esp/esp-matter/connectedhomeip/connectedhomeip/src/lib/core/CHIPCore.h:30,
                 from /home/ssafy/esp/esp-matter/connectedhomeip/connectedhomeip/src/app/MessageDef/InvokeRequestMessage.h:22,
                 from /home/ssafy/esp/esp-matter/connectedhomeip/connectedhomeip/src/app/CommandSender.h:30,
                 from /home/ssafy/esp/esp-matter/connectedhomeip/connectedhomeip/src/app/DeviceProxy.h:29,
                 from /home/ssafy/esp/esp-matter/components/esp_matter/esp_matter_core.h:17,
                 from /home/ssafy/esp/esp-matter/components/esp_matter/esp_matter_attribute.h:18,
                 from /home/ssafy/esp/esp-matter/components/esp_matter/esp_matter.h:22,
                 from /home/ssafy/esp/esp-matter/examples/dimmable_tflite/main/app_main.cpp:13:
/home/ssafy/esp/esp-matter/connectedhomeip/connectedhomeip/src/lib/support/CodeUtils.h:705: note: macro "ArraySize" defined here
  705 | #define ArraySize(a) (sizeof(a) / sizeof((a)[0]))
      |
/home/ssafy/esp/esp-matter/examples/dimmable_tflite/managed_components/espressif__esp-tflite-micro/tensorflow/lite/kernels/internal/types.h:252:72: error: macro "ArraySize" passed 2 arguments, but takes just 1
  252 |   TFLITE_DCHECK_EQ(ArraySize(array1, index1), ArraySize(array2, index2));
      |                                                                        ^
/home/ssafy/esp/esp-matter/connectedhomeip/connectedhomeip/src/lib/support/CodeUtils.h:705: note: macro "ArraySize" defined here
  705 | #define ArraySize(a) (sizeof(a) / sizeof((a)[0]))
      |
/home/ssafy/esp/esp-matter/examples/dimmable_tflite/managed_components/espressif__esp-tflite-micro/tensorflow/lite/kernels/internal/types.h:252:72: error: macro "ArraySize" passed 2 arguments, but takes just 1
  252 |   TFLITE_DCHECK_EQ(ArraySize(array1, index1), ArraySize(array2, index2));
      |                                                                        ^

Platform

other, core (please add to version below)

Platform Version(s)

No response

Type

Manually tested with SDK

(Optional) If manually tested please explain why this is only manually tested

No response

Anything else?

No response

bzbarsky-apple commented 8 months ago

I don't think enclosing a macro in a namespace does anything...

@dhrishi @shubhamdp can you take a look, since this is an esp-matter issue?

andy31415 commented 8 months ago

If this is on ESP32, why is a silabs file connectedhomeip/third_party/silabs/gecko_sdk/util/third_party/tflite-micro/tensorflow/lite/kernels/internal/types.h included?

andy31415 commented 8 months ago

If this is on ESP32, why is a silabs file connectedhomeip/third_party/silabs/gecko_sdk/util/third_party/tflite-micro/tensorflow/lite/kernels/internal/types.h included?

Guessing summary mentions wrong file? @bmryu0501 could you update the summary? I think the type redefinition is somewhere else.

andy31415 commented 8 months ago

I believe we should probably rename macros as ALL_UPPER_SNAKE_CASE since it seems that using ArraySize looks too much like a function. We should have ARRAY_SIZE or CHIP_ARRAY_SIZE or similar instead. I believe that is the standard for macros (even though some things like ChipLog.... macros break that, but at least they are prefix-named with chip.

bmryu0501 commented 8 months ago

If this is on ESP32, why is a silabs file connectedhomeip/third_party/silabs/gecko_sdk/util/third_party/tflite-micro/tensorflow/lite/kernels/internal/types.h included?

Guessing summary mentions wrong file? @bmryu0501 could you update the summary? I think the type redefinition is somewhere else.

Thank you for your prompt reply. I realized that idf.py pull codes in managed_components, after the issue was created.

I added dependency using idf.py add-dependency "esp-tflite-micro". After build, types.h is in /home/ssafy/esp/esp-matter/examples/dimmable_tflite/managed_components/espressif__esp-tflite-micro/tensorflow/lite/kernels/internal/types.h. (dimmable_tfilte is my own project folder)

Then the problem occurred, and to resolve it, I moved espressif__esp-tflite-micro into components folder and I changed all ArraySize from tflite to ArraySize_tf in compoents folder.

As of now, the build has been successful and the problem has been temporarily resolved.