search

LIFsCode / ELOC-3.0

Firmware for ELOC 3.0 Hardware
MIT License
3 stars 3 forks source link

Re-arrange file layout #88

Closed OOHehir closed 5 months ago

OOHehir commented 6 months ago

@LIFsCode When you get a chance could you please try building this branch. A 'full clean' before you do is probably best. I've managed to re-arrange the layout of the files & it builds OK on my system, just want to check it's the same for you before I update & merge it into master.

LIFsCode commented 6 months ago

@OOHehir Actually it's not compiling:


Processing esp32dev-ei (platform: espressif32@~6.1.0; board: esp32dev; framework: espidf, arduino)
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------Library Manager: Installing bblanchon/ArduinoJson @ ^6.21.2
Unpacking  [####################################]  100%
Library Manager: ArduinoJson@6.21.5 has been installed!
Library Manager: Installing pvizeli/CmdParser @ 0.0.0-alpha+sha.dcaa7eada9
Unpacking  [####################################]  100%
Library Manager: CmdParser@0.0.0-alpha+sha.dcaa7eada9 has been installed!
Library Manager: Installing evert-arias/EasyBuzzer @ 1.0.4
Unpacking  [####################################]  100%
Library Manager: EasyBuzzer@1.0.4 has been installed!
Verbose mode can be enabled via `-v, --verbose` option
Build Hash: 7e292ebb

<SCons.Script.SConscript.SConsEnvironment object at 0x000002537CE3B390>
Platform:
C:\Users\Fabi\.platformio\packages\framework-espidf@3.40404.0
C:\Users\Fabi\.platformio\penv\.espidf-4.4.4\Scripts\python.exe
elocPartitions.csv
C:\Users\Fabi\.platformio\packages\framework-espidf@3.40404.0/components/nvs_flash/nvs_partition_generator/nvs_partition_gen.py
['# Name', '   Type', ' SubType', ' Offset', '  Size', ' Flags']
['nvs', '        data', '   nvs', '        0x9000', '     0x5000', '']
NVS base:         0x9000
['otadata', '    data', '   ota', '        0xe000', '     0x2000', '']
['partition0', ' app', '    ota_0', '      0x10000', '    0x7E0000', '']
partition0 base:       0x10000
['partition1', ' app', '    ota_1', '      0x7F0000', '   0x7E0000', '']
['spiffs', '     data', '   spiffs', '     0xFD0000', '   0x30000', '']

Creating NVS binary with version: V2 - Multipage Blob Support Enabled

Created NVS binary: ===> E:\Elektronik\ElephantProject\ELOC-3.0_AI\eloc610LowPowerPartition\.pio/build/esp32dev/nvs.bin
NVS generated successuflly
CONFIGURATION: https://docs.platformio.org/page/boards/espressif32/esp32dev.html
PLATFORM: Espressif 32 (6.1.0) > Espressif ESP32 Dev Module
HARDWARE: ESP32 240MHz, 320KB RAM, 4MB Flash
DEBUG: Current (esp-prog) External (cmsis-dap, esp-bridge, esp-prog, iot-bus-jtag, jlink, minimodule, olimex-arm-usb-ocd, olimex-arm-usb-ocd-h, olimex-arm-usb-tiny-h, olimex-jtag-tiny, tumpa)
PACKAGES:
 - framework-arduinoespressif32 @ 3.20007.0 (2.0.7)
 - framework-espidf @ 3.40404.0 (4.4.4)
 - tool-cmake @ 3.16.4
 - tool-esptoolpy @ 1.40500.0 (4.5.0)
 - tool-idf @ 1.0.1
 - tool-mconf @ 1.4060000.20190628 (406.0.0)
 - tool-ninja @ 1.9.0
 - toolchain-esp32ulp @ 1.23500.220830 (2.35.0) 
 - toolchain-xtensa-esp32 @ 8.4.0+2021r2-patch5
Warning! Arduino framework as an ESP-IDF component doesn't handle the `variant` field! The default `esp32` variant will be used.
Reading CMake configuration...
Generating assembly for certificate bundle...
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 32 compatible libraries
Scanning dependencies...
Dependency Graph
|-- ArduinoJson @ 6.21.5
|-- CmdParser @ 0.0.0-alpha+sha.dcaa7eada9
|-- EasyBuzzer @ 1.0.4
|-- CPPANALOGIO
|-- BluetoothServer
|-- edge-impulse
|-- ElocConfig
|-- ElocStatus
|-- ElocSystem
|-- ESP32Time @ 1.0.3
|-- ffsutils
|-- FirmwareUpdate
|-- audio_input
|-- Accel
|-- logging
|-- PerfMonitor
|-- sd_card
|-- spiffs
|-- time_utils
|-- uart_test
|-- wav_file
Building in release mode
Compiling .pio\build\esp32dev-ei\src\main.o
Generating LD script .pio\build\esp32dev-ei\memory.ld
Compiling .pio\build\esp32dev-ei\app_trace\app_trace.o
Compiling .pio\build\esp32dev-ei\app_trace\app_trace_util.o
Compiling .pio\build\esp32dev-ei\app_trace\host_file_io.o
Compiling .pio\build\esp32dev-ei\app_trace\gcov\gcov_rtio.o
Compiling .pio\build\esp32dev-ei\app_update\esp_ota_ops.o
Compiling .pio\build\esp32dev-ei\app_update\esp_app_desc.o
Archiving .pio\build\esp32dev-ei\esp-idf\app_trace\libapp_trace.a
Indexing .pio\build\esp32dev-ei\esp-idf\app_trace\libapp_trace.a
Compiling .pio\build\esp32dev-ei\asio\asio\asio\src\asio.o
Archiving .pio\build\esp32dev-ei\esp-idf\app_update\libapp_update.a
Compiling .pio\build\esp32dev-ei\bootloader_support\src\bootloader_common.o
Indexing .pio\build\esp32dev-ei\esp-idf\app_update\libapp_update.a
Compiling .pio\build\esp32dev-ei\bootloader_support\src\bootloader_common_loader.o
Compiling .pio\build\esp32dev-ei\bootloader_support\src\bootloader_clock_init.o
In file included from lib/edge-impulse/src/edge-impulse-sdk/dsp/numpy_types.h:33,
                 from lib/edge-impulse/src/EdgeImpulse.hpp:33,
                 from lib/ElocStatus/src/ElocStatus.hpp:40,
                 from lib/ElocSystem/src/ElocSystem.hpp:33,
                 from lib/CPPANALOGIO/src/Battery.hpp:34,
                 from src/main.cpp:45:
lib/edge-impulse/src/edge-impulse-sdk/dsp/../porting/ei_classifier_porting.h:133: warning: "EI_PORTING_ARDUINO" redefined
 #define EI_PORTING_ARDUINO        0

lib/edge-impulse/src/edge-impulse-sdk/dsp/../porting/ei_classifier_porting.h:124: note: this is the location of the previous definition
 #define EI_PORTING_ARDUINO      1

Compiling .pio\build\esp32dev-ei\bootloader_support\src\bootloader_flash.o
Compiling .pio\build\esp32dev-ei\bootloader_support\src\bootloader_mem.o
Compiling .pio\build\esp32dev-ei\bootloader_support\src\bootloader_random.o
src/main.cpp: In function 'void testInput()':
src/main.cpp:182:9: error: 'i2s_mic_Config' was not declared in this scope
         i2s_mic_Config.sample_rate = i;
         ^~~~~~~~~~~~~~
src/main.cpp:182:9: note: suggested alternative: 'i2s_pin_config_t'
         i2s_mic_Config.sample_rate = i;
         ^~~~~~~~~~~~~~
         i2s_pin_config_t
src/main.cpp:185:31: error: 'i2s_mic_pins' was not declared in this scope
         input.init(I2S_NUM_0, i2s_mic_pins, i2s_mic_Config, getMicInfo().MicBitShift, getConfig().listenOnly, getMicInfo().MicUseTimingFix);
                               ^~~~~~~~~~~~
src/main.cpp:185:31: note: suggested alternative: 'i2s_set_pin'
         input.init(I2S_NUM_0, i2s_mic_pins, i2s_mic_Config, getMicInfo().MicBitShift, getConfig().listenOnly, getMicInfo().MicUseTimingFix);
                               ^~~~~~~~~~~~
                               i2s_set_pin
src/main.cpp: In function 'void ei_callback_func()':
src/main.cpp:543:40: warning: missing initializer for member 'ei_impulse_result_t::bounding_boxes_count' [-Wmissing-field-initializers]
         ei_impulse_result_t result = {0};
                                        ^
src/main.cpp:543:40: warning: missing initializer for member 'ei_impulse_result_t::classification' [-Wmissing-field-initializers]
src/main.cpp:543:40: warning: missing initializer for member 'ei_impulse_result_t::anomaly' [-Wmissing-field-initializers]
src/main.cpp:543:40: warning: missing initializer for member 'ei_impulse_result_t::timing' [-Wmissing-field-initializers]
src/main.cpp: In function 'void app_main()':
src/main.cpp:790:40: warning: missing initializer for member 'ei_impulse_result_t::bounding_boxes_count' [-Wmissing-field-initializers]
         ei_impulse_result_t result = {0};
                                        ^
src/main.cpp:790:40: warning: missing initializer for member 'ei_impulse_result_t::classification' [-Wmissing-field-initializers]
src/main.cpp:790:40: warning: missing initializer for member 'ei_impulse_result_t::anomaly' [-Wmissing-field-initializers]
src/main.cpp:790:40: warning: missing initializer for member 'ei_impulse_result_t::timing' [-Wmissing-field-initializers]
src/main.cpp:862:34: error: 'i2s_mic_pins' was not declared in this scope
     input.init(I2S_DEFAULT_PORT, i2s_mic_pins, i2s_mic_Config, getMicInfo().MicBitShift,
                                  ^~~~~~~~~~~~
src/main.cpp:862:34: note: suggested alternative: 'i2s_set_pin'
     input.init(I2S_DEFAULT_PORT, i2s_mic_pins, i2s_mic_Config, getMicInfo().MicBitShift,
                                  ^~~~~~~~~~~~
                                  i2s_set_pin
src/main.cpp:862:48: error: 'i2s_mic_Config' was not declared in this scope
     input.init(I2S_DEFAULT_PORT, i2s_mic_pins, i2s_mic_Config, getMicInfo().MicBitShift,
                                                ^~~~~~~~~~~~~~
src/main.cpp:862:48: note: suggested alternative: 'i2s_pin_config_t'
     input.init(I2S_DEFAULT_PORT, i2s_mic_pins, i2s_mic_Config, getMicInfo().MicBitShift,
                                                ^~~~~~~~~~~~~~
                                                i2s_pin_config_t
In file included from lib/audio_input/src/I2SMEMSSampler.h:4,
                 from src/main.cpp:13:
lib/wav_file/src/WAVFileWriter.h: At global scope:
lib/wav_file/src/WAVFileWriter.h:24:23: warning: 'wav_static_buffers' defined but not used [-Wunused-variable]
   static signed short wav_static_buffers[2][wav_static_buffer_size];
                       ^~~~~~~~~~~~~~~~~~
*** [.pio\build\esp32dev-ei\src\main.o] Error 1

Are you really sure that this is the way to go? For the whole folder structure is completly confusing.

I thought you would only extract bigger portions which are independently runnable as libraries (e.g. EdgeImpulse, . But this is a single lib for every .cpp file.

Actually in this case I would strongly prefer the original folder structure. Sorry for the turnaround, I really didn't got the full picture into my mind during our call.

OOHehir commented 6 months ago

@LIFsCode OK, how about something in between. Its currently:

├── include
│   ├── ei_inference.h
│   ├── project_config.h
│   ├── README
│   └── test_samples.h
├── lib
│   ├── Accel
│   ├── audio_input
│   ├── buzzer
│   ├── CPPANALOGIO
│   ├── CPPI2C
│   ├── edge-impulse
│   ├── esp32Time
│   ├── IO_expander
│   ├── mock
│   ├── README
│   ├── sd_card
│   ├── spiffs
│   └── wav_file
├── logs
├── nvs.csv
├── platformio.ini
├── README-ai.md
├── readme.txt
├── sdkconfig
├── sdkconfig.defaults
├── src
│   ├── Battery.cpp
│   ├── Battery.hpp
│   ├── CMakeLists.txt
│   ├── Commands
│   ├── config.cpp
│   ├── config.h
│   ├── ElocConfig.cpp
│   ├── ElocConfig.hpp
│   ├── ElocStatus.cpp
│   ├── ElocStatus.hpp
│   ├── ElocSystem.cpp
│   ├── ElocSystem.hpp
│   ├── main.cpp
│   ├── PerfMonitor.cpp
│   ├── PerfMonitor.hpp
│   ├── utils
│   └── version.h
├── test

How about (roughly)

── include
│   ├── ei_inference.h
│   ├── project_config.h
│   ├── README
│   └── test_samples.h
├── lib
│   ├── Accel
│   ├── audio_input
│   ├── buzzer
│   ├── CPPANALOGIO
│   ├── CPPI2C
│   ├── edge-impulse
│   ├── esp32Time
│   ├── IO_expander
│   ├── mock
│   ├── README
│   ├── sd_card
│   ├── FirmwareUpdate
│   ├── PerfMonitor
│   ├── Battery
│   ├── Commands
│   ├── sd_card
│   ├── spiffs
│   └── wav_file
├── nvs.csv
├── platformio.ini
├── README-ai.md
├── readme.txt
├── sdkconfig
├── sdkconfig.defaults
├── src
│   ├── CMakeLists.txt
│   ├── config.cpp
│   ├── config.h
│   ├── ElocConfig.cpp
│   ├── ElocConfig.hpp
│   ├── ElocStatus.cpp
│   ├── ElocStatus.hpp
│   ├── ElocSystem.cpp
│   ├── ElocSystem.hpp
│   ├── main.cpp
│   ├── utils
│   └── version.h
├── test
LIFsCode commented 6 months ago

@OOHehir yes that looks much better :) Keeeping the whole command interface together makes sense they rely heavily on each other.

Just a few slight modifications. I think we can move the utils folder to lib folder it has no dependencies to anything else:

── include
│   ├── ei_inference.h
│   ├── project_config.h
│   ├── README
│   └── test_samples.h
├── lib
│   ├── Accel
│   ├── audio_input
│   ├── buzzer
│   ├── CPPANALOGIO
│   ├── CPPI2C
│   ├── edge-impulse
│   ├── esp32Time
│   ├── IO_expander
│   ├── mock
│   ├── README
│   ├── sd_card
│   ├── FirmwareUpdate
│   ├── PerfMonitor
│   ├── Battery
│   ├── Commands
│   ├── spiffs
│   ├── utils
│   └── wav_file
├── nvs.csv
├── platformio.ini
├── README-ai.md
├── readme.txt
├── sdkconfig
├── sdkconfig.defaults
├── src
│   ├── CMakeLists.txt
│   ├── config.cpp
│   ├── config.h
│   ├── ElocConfig.cpp
│   ├── ElocConfig.hpp
│   ├── ElocStatus.cpp
│   ├── ElocStatus.hpp
│   ├── ElocSystem.cpp
│   ├── ElocSystem.hpp
│   ├── main.cpp
│   └── version.h
├── test

Besides that I just came across a few things:

LIFsCode commented 6 months ago

@OOHehir ah maybe one more thing:

Could you please keep the EdgeImpulse.cpp/.hpp untouched in the meantime. I'm currently working on a major refactoring which also includes moving the edge impluse functions from main.cpp into EdgeImpulse.cpp and its class. It would be much easier to merge it with this layout rearrangement if the EdgeImpulse remains in place at first.

I would move the EdgeImpulse.cpp myself once I have finished the ProcessFactory and merged everything into main if that's ok for you.

OOHehir commented 6 months ago

@LIFsCode Sure, no problem.

With regards EdgeImpulse, you may wish to keep in mind that the files in /lib/edge-impulse/src are subject to complete replacement when the model changes.

OOHehir commented 6 months ago

@LIFsCode @EDsteve I've just pushed a significant re-arrangement to the folder structure. I've tried both the EI & non-EI build on my system & seems ok. Hopefully it'll build ok your Windows system also. Its a painful process to rearrange but should help with tests.

Please note a 'Full Clean' is probably advisable before 'Build'

The structure is now:

├── include
│   ├── ei_inference.h
│   ├── project_config.h
│   ├── README
│   └── test_samples.h
├── lib
│   ├── Accel
│   ├── audio_input
│   ├── buzzer
│   ├── Commands
│   ├── config
│   ├── CPPANALOGIO_Battery
│   ├── CPPI2C
│   ├── edge-impulse
│   ├── ElocConfig
│   ├── ElocStatus
│   ├── ElocSystem
│   ├── esp32Time
│   ├── FirmwareUpdate
│   ├── IO_expander
│   ├── mock
│   ├── PerfMonitor
│   ├── README
│   ├── sd_card
│   ├── spiffs
│   ├── utils
│   └── wav_file
├── logs
├── nvs.csv
├── platformio.ini
├── README-ai.md
├── readme.txt
├── sdkconfig
├── sdkconfig.defaults
├── src
│   ├── CMakeLists.txt
│   ├── main.cpp
│   └── version.h

@LIFsCode I had to modify EdgeImpulse by a few lines, apologies, but I would have to reverse all the changes without doing so. I had to include ElocSystem, ElocStatus & ElocConif into lib also as they have dependencies that break without moving them.

LIFsCode commented 6 months ago

@OOHehir no problem I will get along with it

Would it be possible to have the Eloc* (status, config, system) into a single lib? ElocConfig may be tested standalone but the rest is highly dependent.

I find it a more clear to read if at least all the Eloc specific stuff is kept together.

OOHehir commented 6 months ago

@LIFsCode Thanks. Yes, will modify the layout. Any thoughts on the name for the containing folder?

OOHehir commented 6 months ago

@LIFsCode I've called it 'ElocHardware' for the moment

EDsteve commented 6 months ago

@OOHehir I am getting a compile error with 10d83ee1663e22cf0433ff0c63fb836f47336d3d Compile error 10d83ee1.txt

OOHehir commented 6 months ago

@EDsteve Thanks for the report, its similar to @LIFsCode report above.

I notice in your build it looks like this

Dependency Graph
|-- ArduinoJson @ 6.21.5
|-- CmdParser @ 0.0.0-alpha+sha.dcaa7eada9
|-- EasyBuzzer @ 1.0.4
|-- CPPANALOGIO_Battery
|-- Commands
|-- edge-impulse
|-- ElocHardware
|-- ESP32Time @ 1.0.3
|-- utils
|-- FirmwareUpdate
|-- audio_input
|-- Accel
|-- PerfMonitor
|-- sd_card
|-- spiffs
|-- uart_eloc
|-- wav_file

Whereas mine is

Dependency Graph
|-- ArduinoJson @ 6.21.5
|-- CmdParser @ 0.0.0-alpha+sha.dcaa7eada9
|-- EasyBuzzer @ 1.0.4
|-- CPPANALOGIO_Battery
|-- Commands
|-- ESP32Time @ 1.0.3
|-- edge-impulse
|-- ElocHardware
|-- FirmwareUpdate
|-- audio_input
|-- PerfMonitor
|-- sd_card
|-- spiffs
|-- wav_file
|-- config           <------------------------
|-- utils
|-- Accel
|-- uart_eloc

Would you mind deleting Directory 'ELOC-3.0/eloc610LowPowerPartition/.pio' File 'ELOC-3.0/eloc610LowPowerPartition/.vscode/c_cpp_properties.json'

These will get rebuild during the compilation process

EDsteve commented 6 months ago

@OOHehir Unfortunately that didn't work. Same error. This is the new log: Compile error 10d83ee1_2.txt

LIFsCode commented 6 months ago

@OOHehir I don't see any benefit in having the config.cpp/.h in a separate lib folder. It is only a set of constant and/or globals definitions. There could be no suitable unittest running on this alone.

So I guess this can bei either moved to ElocHardware or to the main src folder

OOHehir commented 6 months ago

@LIFsCode I've just moved them into ElocHardware, moving them into main/src breaks the build on my side as EloCConfig.cpp requires the structs.

Would you mind trying the to build the latest? It always built on my desktop & laptop so hard for me to test. Out of interest I'm using PlatformIO Core 6.1.13 Home 3.4.4, are you something similar?

If this still breaks your build I could wrap it in a getter.

As an aside the code below is in main.cpp & now copied into the audio_input unit test, should it be removed from main.cpp? @EDsteve do you use this test code in main.cpp?

void testInput()
{
    ESP_LOGV(TAG, "Func: %s", __func__);

    // TODO: This is now in unit tests (test_target_audio_input). Should it be removed?

    for (uint32_t i = 1000; i < 34000; i = i + 2000) {
        i2s_mic_Config.sample_rate = i;
        i2s_mic_Config.use_apll = getMicInfo().MicUseAPLL;

        input.init(I2S_NUM_0, i2s_mic_pins, i2s_mic_Config, getMicInfo().MicBitShift, getConfig().listenOnly, getMicInfo().MicUseTimingFix);
        input.install_and_start();
        delay(100);
        ESP_LOGI(TAG, "Clockrate: %f", i2s_get_clk(I2S_NUM_0));
        input.uninstall();
        delay(100);
    }

    delay(100);
}
OOHehir commented 5 months ago

@EDsteve @LIFsCode It seems like this is compiling OK on windows machines?

I'll close, please re-open if this is incorrect.