From ef957e5e1e000deeee7b4a05ddee5a1428a0b9da Mon Sep 17 00:00:00 2001 From: Barabas Raffai Date: Mon, 27 Nov 2023 22:17:22 +0000 Subject: [PATCH] Implement some JSON, fix tests --- .gitmodules | 3 + firmware/components/CMakeLists.txt | 4 +- .../components/midi_mapping/CMakeLists.txt | 1 + .../midi_mapping/include/midi_mapping.h | 3 +- .../include/midi_mapping_json_builder.h | 3 - .../include/midi_mapping_json_parser.h | 2 +- .../src/midi_mapping_json_builder.cpp | 21 +--- .../src/midi_mapping_json_parser.cpp | 76 +----------- firmware/components/presets/CMakeLists.txt | 20 ++- firmware/components/presets/include/presets.h | 3 +- .../presets/include/selected_preset_api.h | 9 +- .../include/selected_preset_json_builder.h | 38 ++++++ .../src/selected_preset_json_builder.cpp | 22 ++++ .../test/selected_preset_json_builder.cpp | 81 ++++++++++++ firmware/components/uuid/CMakeLists.txt | 11 ++ firmware/components/uuid/include/uuid.h | 32 +++++ firmware/components/uuid/src/uuid_json.cpp | 116 ++++++++++++++++++ test/.idea/discord.xml | 7 ++ test/.idea/vcs.xml | 2 + test/support/CMakeLists.txt | 1 + test/support/log/include/esp_log.h | 2 + test/support/protobuf-c/CMakeLists.txt | 5 + test/support/protobuf-c/protobuf-c | 1 + 23 files changed, 357 insertions(+), 106 deletions(-) create mode 100644 firmware/components/presets/include/selected_preset_json_builder.h create mode 100644 firmware/components/presets/src/selected_preset_json_builder.cpp create mode 100644 firmware/components/presets/test/selected_preset_json_builder.cpp create mode 100644 firmware/components/uuid/CMakeLists.txt create mode 100644 firmware/components/uuid/include/uuid.h create mode 100644 firmware/components/uuid/src/uuid_json.cpp create mode 100644 test/.idea/discord.xml create mode 100644 test/support/protobuf-c/CMakeLists.txt create mode 160000 test/support/protobuf-c/protobuf-c diff --git a/.gitmodules b/.gitmodules index 80ceaee6..afab82ed 100644 --- a/.gitmodules +++ b/.gitmodules @@ -16,3 +16,6 @@ [submodule "test/support/freertos/FreeRTOS-Kernel"] path = test/support/freertos/FreeRTOS-Kernel url = https://github.com/Barabas5532/FreeRTOS-Kernel.git +[submodule "test/support/protobuf-c/protobuf-c"] + path = test/support/protobuf-c/protobuf-c + url = https://github.com/protobuf-c/protobuf-c.git diff --git a/firmware/components/CMakeLists.txt b/firmware/components/CMakeLists.txt index aa674ce3..66b47992 100644 --- a/firmware/components/CMakeLists.txt +++ b/firmware/components/CMakeLists.txt @@ -12,4 +12,6 @@ add_subdirectory(messages) add_subdirectory(midi_mapping) add_subdirectory(midi_protocol) add_subdirectory(os) -add_subdirectory(persistence) \ No newline at end of file +add_subdirectory(persistence) +add_subdirectory(presets) +add_subdirectory(uuid) diff --git a/firmware/components/midi_mapping/CMakeLists.txt b/firmware/components/midi_mapping/CMakeLists.txt index 7655641f..584c4e2e 100644 --- a/firmware/components/midi_mapping/CMakeLists.txt +++ b/firmware/components/midi_mapping/CMakeLists.txt @@ -15,6 +15,7 @@ target_link_libraries(midi_mapping shrapnel::audio_param shrapnel::etl shrapnel::midi_protocol + shrapnel::uuid shrapnel::rapidjson PRIVATE shrapnel::compiler_warning_flags) diff --git a/firmware/components/midi_mapping/include/midi_mapping.h b/firmware/components/midi_mapping/include/midi_mapping.h index 297f531b..6720da52 100644 --- a/firmware/components/midi_mapping/include/midi_mapping.h +++ b/firmware/components/midi_mapping/include/midi_mapping.h @@ -252,12 +252,13 @@ #include "audio_param.h" #include "midi_protocol.h" +#include "uuid.h" namespace shrapnel { namespace midi { struct Mapping { - using id_t = std::array; + using id_t = uuid::uuid_t; enum class Mode { diff --git a/firmware/components/midi_mapping/include/midi_mapping_json_builder.h b/firmware/components/midi_mapping/include/midi_mapping_json_builder.h index b73d9f87..0652d63d 100644 --- a/firmware/components/midi_mapping/include/midi_mapping_json_builder.h +++ b/firmware/components/midi_mapping/include/midi_mapping_json_builder.h @@ -49,9 +49,6 @@ rapidjson::Value to_json(rapidjson::Document &document, const Message &object); template<> rapidjson::Value to_json(rapidjson::Document &document, const Mapping &object); -template<> -rapidjson::Value to_json(rapidjson::Document &document, const Mapping::id_t &object); - template<> rapidjson::Value to_json(rapidjson::Document &document, const std::pair &object); diff --git a/firmware/components/midi_mapping/include/midi_mapping_json_parser.h b/firmware/components/midi_mapping/include/midi_mapping_json_parser.h index f29f9232..1e966856 100644 --- a/firmware/components/midi_mapping/include/midi_mapping_json_parser.h +++ b/firmware/components/midi_mapping/include/midi_mapping_json_parser.h @@ -94,7 +94,7 @@ std::optional from_json(const rapidjson::Value &json) { return std::nullopt; } - auto id = from_json(entry.name); + auto id = uuid::from_json(entry.name); if(!id.has_value()) { ESP_LOGE(TAG, "failed to get uuid"); diff --git a/firmware/components/midi_mapping/src/midi_mapping_json_builder.cpp b/firmware/components/midi_mapping/src/midi_mapping_json_builder.cpp index 88d329fb..6e100cfd 100644 --- a/firmware/components/midi_mapping/src/midi_mapping_json_builder.cpp +++ b/firmware/components/midi_mapping/src/midi_mapping_json_builder.cpp @@ -114,23 +114,6 @@ rapidjson::Value to_json(rapidjson::Document &document, const Mapping &object) return json; } -template <> -rapidjson::Value to_json(rapidjson::Document &document, - const Mapping::id_t &object) { - char uuid[37]; - sprintf(uuid, - "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-" - "%02x%02x%02x%02x%02x%02x", - object[0], object[1], object[2], object[3], object[4], object[5], - object[6], object[7], object[8], object[9], object[10], object[11], - object[12], object[13], object[14], object[15]); - - rapidjson::Value out; - out.SetString(uuid, 36, document.GetAllocator()); - - return out; -} - template<> rapidjson::Value to_json(rapidjson::Document &document, const std::pair &object) { @@ -138,7 +121,7 @@ rapidjson::Value to_json(rapidjson::Document &document, const std::pair std::optional from_json(const rapidjson::Value &) { @@ -117,7 +115,7 @@ std::optional> from_json(const rapidjson::Valu return std::nullopt; } - auto id = from_json(mapping_id); + auto id = uuid::from_json(mapping_id); if(!id.has_value()) { ESP_LOGE(TAG, "failed to get id"); return std::nullopt; @@ -166,7 +164,7 @@ std::optional from_json(const rapidjson::Value &json) { return std::nullopt; } - auto uuid = from_json(id_member->value); + auto uuid = uuid::from_json(id_member->value); if(!uuid.has_value()) { ESP_LOGE(TAG, "Failed to parse UUID"); @@ -210,75 +208,5 @@ std::optional from_json(const rapidjson::Value &json) { return std::nullopt; } -template<> -std::optional from_json(const rapidjson::Value &json) { - constexpr const char *TAG = "Mapping::id_t from_json"; - - if (!json.IsString()) { - ESP_LOGE(TAG, "id is not string"); - return std::nullopt; - } - - Mapping::id_t out; - int rc = parse_uuid(out, json.GetString()); - if(rc != 0) { - ESP_LOGE(TAG, "failed to get uuid"); - return std::nullopt; - } - - return out; -} - -static int parse_uuid(Mapping::id_t &uuid, const char *string) -{ - constexpr std::size_t UUID_LENGTH = 36; - constexpr char TAG[] = "parse_uuid"; - - if(UUID_LENGTH != std::strlen(string)) - { - ESP_LOGE(TAG, "Incorrect UUID string length"); - return -1; - } - - size_t i = 0; - size_t j = 0; - ESP_LOGD(TAG, "i = %zu, j = %zu", i , j); - while(i < UUID_LENGTH) - { - char digit[2]; - std::memcpy(digit, &string[i], 2); - - ESP_LOGD(TAG, "digit = %c%c", digit[0] , digit[1]); - - // TODO error on invalid characters: z, symbols etc - // The only valid characters are 0 to 9 and a to f. - // - // Handle uppercase as well. It is required when a string UUID - // is an input as per RFC 4122 Section 3 - // https://tools.ietf.org/html/rfc4122#section-3 - auto get_value = [&] (char hex) -> uint8_t { - if(hex >= 'a') - { - return hex - 'a' + 10; - } - else - { - return hex - '0'; - } - }; - - uuid[j] = get_value(digit[0]) << 4 | get_value(digit[1]); - - j++; - - i += 2; - bool is_separator = (i == 8) || (i == 13) || (i == 18) || (i == 23); - if(is_separator) i++; - ESP_LOGD(TAG, "i = %zu, j = %zu", i , j); - } - - return 0; -} - } } diff --git a/firmware/components/presets/CMakeLists.txt b/firmware/components/presets/CMakeLists.txt index fd508b0f..c99a0068 100644 --- a/firmware/components/presets/CMakeLists.txt +++ b/firmware/components/presets/CMakeLists.txt @@ -4,7 +4,23 @@ add_library(shrapnel::presets ALIAS presets) target_sources(presets PRIVATE proto/generated/presets.pb-c.c - src/presets_json_builder.cpp) + src/presets_json_builder.cpp + src/selected_preset_json_builder.cpp) target_include_directories(presets PUBLIC proto/generated include) -target_link_libraries(presets PUBLIC idf::protobuf-c shrapnel::rapidjson) \ No newline at end of file +target_link_libraries(presets PUBLIC idf::protobuf-c shrapnel::rapidjson shrapnel::etl shrapnel::uuid) + +if(DEFINED TESTING) + add_executable(presets_test + test/selected_preset_json_builder.cpp) + + target_include_directories(presets_test PRIVATE include) + + target_link_libraries(presets_test + PRIVATE + presets + GTest::gtest_main + GTest::gmock + shrapnel::compiler_warning_flags) + gtest_discover_tests(presets_test) +endif() diff --git a/firmware/components/presets/include/presets.h b/firmware/components/presets/include/presets.h index 729301bc..c3204d0e 100644 --- a/firmware/components/presets/include/presets.h +++ b/firmware/components/presets/include/presets.h @@ -1,8 +1,7 @@ #pragma once #include -#include namespace shrapnel::presets { -using id_t = etl::array; +using id_t = std::array; } diff --git a/firmware/components/presets/include/selected_preset_api.h b/firmware/components/presets/include/selected_preset_api.h index 39fe4568..a0078df1 100644 --- a/firmware/components/presets/include/selected_preset_api.h +++ b/firmware/components/presets/include/selected_preset_api.h @@ -4,8 +4,11 @@ #include #include "presets.h" +#include "uuid.h" -namespace shrapnel::presets { +namespace shrapnel::selected_preset { + +using shrapnel::uuid::uuid_t; struct Read { @@ -13,12 +16,12 @@ struct Read struct Notify { - id_t selectedPresetId; + uuid_t selectedPresetId; }; struct Write { - id_t selectedPresetId; + uuid_t selectedPresetId; }; using SelectedPresetApiMessage = std::variant; diff --git a/firmware/components/presets/include/selected_preset_json_builder.h b/firmware/components/presets/include/selected_preset_json_builder.h new file mode 100644 index 00000000..89697fd3 --- /dev/null +++ b/firmware/components/presets/include/selected_preset_json_builder.h @@ -0,0 +1,38 @@ +#pragma once + +// Disable warning inside rapidjson +// https://github.com/Tencent/rapidjson/issues/1700 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wclass-memaccess" +#pragma GCC diagnostic ignored "-Wzero-as-null-pointer-constant" +#pragma GCC diagnostic ignored "-Wsign-conversion" +#pragma GCC diagnostic ignored "-Wswitch-enum" +#include "rapidjson/document.h" +#include "rapidjson/writer.h" +#pragma GCC diagnostic pop + +#include "selected_preset_api.h" + +namespace shrapnel::selected_preset { + +/** Convert \p object to a JSON value + * + * \note The \p document must not by modified by this function. It is only + * passed in so that its allocator member can be accessed. + */ +template +rapidjson::Value to_json(rapidjson::Document &document, const T &object); + +template <> +rapidjson::Value to_json(rapidjson::Document &document, + const Read &object); + +template <> +rapidjson::Value to_json(rapidjson::Document &document, + const Notify &object); + +template <> +rapidjson::Value to_json(rapidjson::Document &document, + const Write &object); + +} // namespace shrapnel::selected_preset \ No newline at end of file diff --git a/firmware/components/presets/src/selected_preset_json_builder.cpp b/firmware/components/presets/src/selected_preset_json_builder.cpp new file mode 100644 index 00000000..9e9761ef --- /dev/null +++ b/firmware/components/presets/src/selected_preset_json_builder.cpp @@ -0,0 +1,22 @@ +#include "selected_preset_json_builder.h" + +namespace shrapnel::selected_preset { + +template <> +rapidjson::Value to_json(rapidjson::Document &document, const Notify &object) +{ + rapidjson::Value json; + json.SetObject(); + + json.AddMember("messageType", + rapidjson::StringRef("SelectedPreset::notify"), + document.GetAllocator()); + + json.AddMember("selectedPreset", + uuid::to_json(document, object.selectedPresetId), + document.GetAllocator()); + + return json; +} + +} // namespace shrapnel::selected_preset \ No newline at end of file diff --git a/firmware/components/presets/test/selected_preset_json_builder.cpp b/firmware/components/presets/test/selected_preset_json_builder.cpp new file mode 100644 index 00000000..6e0bede3 --- /dev/null +++ b/firmware/components/presets/test/selected_preset_json_builder.cpp @@ -0,0 +1,81 @@ +#include "selected_preset_json_builder.h" + +#include +#include + +// Disable warning inside rapidjson +// https://github.com/Tencent/rapidjson/issues/1700 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wclass-memaccess" +#pragma GCC diagnostic ignored "-Wzero-as-null-pointer-constant" +#pragma GCC diagnostic ignored "-Wsign-conversion" +#pragma GCC diagnostic ignored "-Wswitch-enum" +#include "rapidjson/document.h" +#include "rapidjson/prettywriter.h" +#include "rapidjson/stringbuffer.h" +#pragma GCC diagnostic pop + +namespace { + +using namespace shrapnel; +using namespace shrapnel::selected_preset; + +template +std::string write_json(const T &object) +{ + rapidjson::Document document; + auto result = to_json(document, object); + + result.Swap(document); + + rapidjson::StringBuffer buffer; + rapidjson::PrettyWriter writer(buffer); + document.Accept(writer); + + return buffer.GetString(); +} + +std::string normalise_json(const std::string &json) +{ + rapidjson::Document document; + document.Parse(json.c_str()); + + rapidjson::StringBuffer buffer; + rapidjson::PrettyWriter writer(buffer); + document.Accept(writer); + + return buffer.GetString(); +} + +TEST(SelectedPresetJsonBuilder, Notify) +{ + Notify input{.selectedPresetId{uuid::uuid_t{ + 0, + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + }}}; + + rapidjson::Document document; + + auto reference = normalise_json(R"({ + "messageType": "SelectedPreset::notify", + "selectedPreset": "00010203-0405-0607-0809-0a0b0c0d0e0f" + })"); + + EXPECT_THAT(write_json(input), reference); +} + +} // namespace \ No newline at end of file diff --git a/firmware/components/uuid/CMakeLists.txt b/firmware/components/uuid/CMakeLists.txt new file mode 100644 index 00000000..b171c885 --- /dev/null +++ b/firmware/components/uuid/CMakeLists.txt @@ -0,0 +1,11 @@ +add_library(shrapnel_uuid STATIC) +add_library(shrapnel::uuid ALIAS shrapnel_uuid) + +target_include_directories(shrapnel_uuid PUBLIC include) + +target_sources(shrapnel_uuid PRIVATE src/uuid_json.cpp) + +target_link_libraries(shrapnel_uuid + PRIVATE + idf::log + shrapnel::compiler_warning_flags) diff --git a/firmware/components/uuid/include/uuid.h b/firmware/components/uuid/include/uuid.h new file mode 100644 index 00000000..4c518908 --- /dev/null +++ b/firmware/components/uuid/include/uuid.h @@ -0,0 +1,32 @@ +#pragma once + +// Disable warning inside rapidjson +// https://github.com/Tencent/rapidjson/issues/1700 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wclass-memaccess" +#pragma GCC diagnostic ignored "-Wzero-as-null-pointer-constant" +#pragma GCC diagnostic ignored "-Wsign-conversion" +#pragma GCC diagnostic ignored "-Wswitch-enum" +#include "rapidjson/document.h" +#pragma GCC diagnostic pop + +#include +#include +#include + +namespace shrapnel::uuid { +using uuid_t = std::array; + +template +std::optional from_json(const rapidjson::Value &json); + +template <> +std::optional from_json(const rapidjson::Value &json); + +template +rapidjson::Value to_json(rapidjson::Document &document, const T &object); + +template <> +rapidjson::Value to_json(rapidjson::Document &document, const uuid_t &object); + +} // namespace shrapnel::uuid \ No newline at end of file diff --git a/firmware/components/uuid/src/uuid_json.cpp b/firmware/components/uuid/src/uuid_json.cpp new file mode 100644 index 00000000..e72e1bbe --- /dev/null +++ b/firmware/components/uuid/src/uuid_json.cpp @@ -0,0 +1,116 @@ +#include "uuid.h" + +#include "esp_log.h" +#include +#include + +namespace shrapnel::uuid { + +static int parse_uuid(uuid_t &uuid, const char *string); + +template <> +std::optional from_json(const rapidjson::Value &json) +{ + constexpr const char *TAG = "Mapping::id_t from_json"; + + if(!json.IsString()) + { + ESP_LOGE(TAG, "id is not string"); + return std::nullopt; + } + + uuid_t out; + int rc = parse_uuid(out, json.GetString()); + if(rc != 0) + { + ESP_LOGE(TAG, "failed to get uuid"); + return std::nullopt; + } + + return out; +} + +static int parse_uuid(uuid_t &uuid, const char *string) +{ + constexpr std::size_t UUID_LENGTH = 36; + constexpr char TAG[] = "parse_uuid"; + + if(UUID_LENGTH != std::strlen(string)) + { + ESP_LOGE(TAG, "Incorrect UUID string length"); + return -1; + } + + size_t i = 0; + size_t j = 0; + ESP_LOGD(TAG, "i = %zu, j = %zu", i, j); + while(i < UUID_LENGTH) + { + char digit[2]; + std::memcpy(digit, &string[i], 2); + + ESP_LOGD(TAG, "digit = %c%c", digit[0], digit[1]); + + // TODO error on invalid characters: z, symbols etc + // The only valid characters are 0 to 9 and a to f. + // + // Handle uppercase as well. It is required when a string UUID + // is an input as per RFC 4122 Section 3 + // https://tools.ietf.org/html/rfc4122#section-3 + auto get_value = [&](char hex) -> uint8_t + { + if(hex >= 'a') + { + return hex - 'a' + 10; + } + else + { + return hex - '0'; + } + }; + + uuid[j] = get_value(digit[0]) << 4 | get_value(digit[1]); + + j++; + + i += 2; + bool is_separator = (i == 8) || (i == 13) || (i == 18) || (i == 23); + if(is_separator) + i++; + ESP_LOGD(TAG, "i = %zu, j = %zu", i, j); + } + + return 0; +} + +template <> +rapidjson::Value to_json(rapidjson::Document &document, const uuid_t &object) +{ + char uuid[37]; + sprintf(uuid, + "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-" + "%02x%02x%02x%02x%02x%02x", + object[0], + object[1], + object[2], + object[3], + object[4], + object[5], + object[6], + object[7], + object[8], + object[9], + object[10], + object[11], + object[12], + object[13], + object[14], + object[15]); + + rapidjson::Value out; + out.SetString(uuid, 36, document.GetAllocator()); + + return out; +} + +} // namespace shrapnel::uuid \ No newline at end of file diff --git a/test/.idea/discord.xml b/test/.idea/discord.xml new file mode 100644 index 00000000..30bab2ab --- /dev/null +++ b/test/.idea/discord.xml @@ -0,0 +1,7 @@ + + + + + \ No newline at end of file diff --git a/test/.idea/vcs.xml b/test/.idea/vcs.xml index c492a188..4696eac1 100644 --- a/test/.idea/vcs.xml +++ b/test/.idea/vcs.xml @@ -3,5 +3,7 @@ + + \ No newline at end of file diff --git a/test/support/CMakeLists.txt b/test/support/CMakeLists.txt index 737b2b41..88b69ac7 100644 --- a/test/support/CMakeLists.txt +++ b/test/support/CMakeLists.txt @@ -3,3 +3,4 @@ add_subdirectory(esp_common) add_subdirectory(esp_hw_support) add_subdirectory(freertos) add_subdirectory(log) +add_subdirectory(protobuf-c) diff --git a/test/support/log/include/esp_log.h b/test/support/log/include/esp_log.h index 0615116a..a97bc650 100644 --- a/test/support/log/include/esp_log.h +++ b/test/support/log/include/esp_log.h @@ -79,6 +79,8 @@ uint32_t esp_log_timestamp(void); void esp_log_write(esp_log_level_t level, const char* tag, const char* format, ...) __attribute__ ((format (printf, 3, 4))); #define ESP_LOG_BUFFER_HEX_LEVEL( tag, buffer, buff_len, level ) +#define ESP_LOG_BUFFER_HEX(tag, buffer, buff_len) \ + ESP_LOG_BUFFER_HEX_LEVEL(tag, buffer, buff_len, ESP_LOG_INFO) #ifdef __cplusplus } diff --git a/test/support/protobuf-c/CMakeLists.txt b/test/support/protobuf-c/CMakeLists.txt new file mode 100644 index 00000000..7b6d49d0 --- /dev/null +++ b/test/support/protobuf-c/CMakeLists.txt @@ -0,0 +1,5 @@ +add_library(idf_protobuf-c STATIC) +add_library(idf::protobuf-c ALIAS idf_protobuf-c) + +target_sources(idf_protobuf-c PRIVATE protobuf-c/protobuf-c/protobuf-c.c) +target_include_directories(idf_protobuf-c PUBLIC protobuf-c) diff --git a/test/support/protobuf-c/protobuf-c b/test/support/protobuf-c/protobuf-c new file mode 160000 index 00000000..abc67a11 --- /dev/null +++ b/test/support/protobuf-c/protobuf-c @@ -0,0 +1 @@ +Subproject commit abc67a11c6db271bedbb9f58be85d6f4e2ea8389