Skip to content

Commit 18e7b08

Browse files
elbenoelbeno
committed
🎨 Refactor string catalog generation
Problem: - Ad-hoc data structures in `gen_str_catalog.py` are unwieldy. - Enum lookup was broken by encoding change with `encode_32` etc. Solution: - Introduction python classes for `Message` and `Module`; simplify code. - Add some tests for `gen_str_catalog.py`. - Fix enum lookup: include enum info inside `encode_32<T>`.
1 parent 3644dba commit 18e7b08

File tree

7 files changed

+332
-170
lines changed

7 files changed

+332
-170
lines changed

‎.github/workflows/unit_tests.yml‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,12 @@ jobs:
128128
${{ matrix.install }}
129129
sudo apt install -y ninja-build python3-venv python3-pip
130130
131-
- name: Install python requirements for string catalog
131+
- name: Install python requirements
132132
run: |
133133
python3 -m venv ${{github.workspace}}/test_venv
134134
source ${{github.workspace}}/test_venv/bin/activate
135135
pip install -r ${{github.workspace}}/tools/requirements.txt
136+
pip install -r ${{github.workspace}}/requirements.txt
136137
echo "${{github.workspace}}/test_venv/bin" >> $GITHUB_PATH
137138
138139
- name: Restore CPM cache
@@ -214,11 +215,12 @@ jobs:
214215
${{ matrix.install }}
215216
sudo apt install -y ninja-build python3-venv python3-pip
216217
217-
- name: Install python requirements for string catalog
218+
- name: Install python requirements
218219
run: |
219220
python3 -m venv ${{github.workspace}}/test_venv
220221
source ${{github.workspace}}/test_venv/bin/activate
221222
pip install -r ${{github.workspace}}/tools/requirements.txt
223+
pip install -r ${{github.workspace}}/requirements.txt
222224
echo "${{github.workspace}}/test_venv/bin" >> $GITHUB_PATH
223225
224226
- name: Restore CPM cache
@@ -341,11 +343,12 @@ jobs:
341343
${{ matrix.install }}
342344
sudo apt install -y ninja-build python3-venv python3-pip
343345
344-
- name: Install python requirements for string catalog
346+
- name: Install python requirements
345347
run: |
346348
python3 -m venv ${{github.workspace}}/test_venv
347349
source ${{github.workspace}}/test_venv/bin/activate
348350
pip install -r ${{github.workspace}}/tools/requirements.txt
351+
pip install -r ${{github.workspace}}/requirements.txt
349352
echo "${{github.workspace}}/test_venv/bin" >> $GITHUB_PATH
350353
351354
- name: Restore CPM cache
@@ -394,11 +397,12 @@ jobs:
394397
run: |
395398
sudo apt update && sudo apt install -y gcc-${{env.DEFAULT_GCC_VERSION}} g++-${{env.DEFAULT_GCC_VERSION}} ninja-build python3-venv python3-pip valgrind
396399
397-
- name: Install python requirements for string catalog
400+
- name: Install python requirements
398401
run: |
399402
python3 -m venv ${{github.workspace}}/test_venv
400403
source ${{github.workspace}}/test_venv/bin/activate
401404
pip install -r ${{github.workspace}}/tools/requirements.txt
405+
pip install -r ${{github.workspace}}/requirements.txt
402406
echo "${{github.workspace}}/test_venv/bin" >> $GITHUB_PATH
403407
404408
- name: Restore CPM cache

‎include/log/catalog/catalog.hpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ using module_id = std::uint32_t;
1616
template <typename> extern auto catalog() -> string_id;
1717
template <typename> extern auto module() -> module_id;
1818

19-
struct encode_32;
20-
struct encode_64;
21-
struct encode_u32;
22-
struct encode_u64;
19+
template <typename> struct encode_32;
20+
template <typename> struct encode_64;
21+
template <typename> struct encode_u32;
22+
template <typename> struct encode_u64;

‎include/log/catalog/mipi_builder.hpp‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,39 +16,33 @@
1616

1717
namespace logging::mipi {
1818
template <typename T>
19-
concept signed_packable =
20-
std::signed_integral<T> and sizeof(T) <= sizeof(std::int64_t);
19+
concept signed_packable = std::signed_integral<stdx::underlying_type_t<T>> and
20+
sizeof(T) <= sizeof(std::int64_t);
2121

2222
template <typename T>
2323
concept unsigned_packable =
24-
std::unsigned_integral<T> and sizeof(T) <= sizeof(std::int64_t);
24+
std::unsigned_integral<stdx::underlying_type_t<T>> and
25+
sizeof(T) <= sizeof(std::int64_t);
2526

2627
template <typename T>
27-
concept enum_packable = std::is_enum_v<T> and sizeof(T) <= sizeof(std::int32_t);
28-
29-
template <typename T>
30-
concept packable =
31-
signed_packable<T> or unsigned_packable<T> or enum_packable<T>;
28+
concept packable = signed_packable<T> or unsigned_packable<T>;
3229

3330
template <typename T> struct encoding;
3431

3532
template <signed_packable T> struct encoding<T> {
3633
using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
37-
encode_32, encode_64>;
34+
encode_32<T>, encode_64<T>>;
3835
using pack_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
3936
std::int32_t, std::int64_t>;
4037
};
4138

4239
template <unsigned_packable T> struct encoding<T> {
4340
using encode_t = stdx::conditional_t<sizeof(T) <= sizeof(std::int32_t),
44-
encode_u32, encode_u64>;
41+
encode_u32<T>, encode_u64<T>>;
4542
using pack_t = stdx::conditional_t<sizeof(T) <= sizeof(std::uint32_t),
4643
std::uint32_t, std::uint64_t>;
4744
};
4845

49-
template <enum_packable T>
50-
struct encoding<T> : encoding<stdx::underlying_type_t<T>> {};
51-
5246
template <packable T> using pack_as_t = typename encoding<T>::pack_t;
5347
template <packable T> using encode_as_t = typename encoding<T>::encode_t;
5448

‎test/log/encoder.cpp‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -146,17 +146,18 @@ TEST_CASE("argument packing", "[mipi]") {
146146
}
147147

148148
TEST_CASE("argument encoding", "[mipi]") {
149+
static_assert(std::same_as<logging::mipi::encode_as_t<std::int32_t>,
150+
encode_32<std::int32_t>>);
151+
static_assert(std::same_as<logging::mipi::encode_as_t<std::uint32_t>,
152+
encode_u32<std::uint32_t>>);
153+
static_assert(std::same_as<logging::mipi::encode_as_t<std::int64_t>,
154+
encode_64<std::int64_t>>);
155+
static_assert(std::same_as<logging::mipi::encode_as_t<std::uint64_t>,
156+
encode_u64<std::uint64_t>>);
149157
static_assert(
150-
std::same_as<logging::mipi::encode_as_t<std::int32_t>, encode_32>);
151-
static_assert(
152-
std::same_as<logging::mipi::encode_as_t<std::uint32_t>, encode_u32>);
153-
static_assert(
154-
std::same_as<logging::mipi::encode_as_t<std::int64_t>, encode_64>);
155-
static_assert(
156-
std::same_as<logging::mipi::encode_as_t<std::uint64_t>, encode_u64>);
157-
static_assert(std::same_as<logging::mipi::encode_as_t<char>, encode_32>);
158-
static_assert(
159-
std::same_as<logging::mipi::encode_as_t<unsigned char>, encode_u32>);
158+
std::same_as<logging::mipi::encode_as_t<char>, encode_32<char>>);
159+
static_assert(std::same_as<logging::mipi::encode_as_t<unsigned char>,
160+
encode_u32<unsigned char>>);
160161
}
161162

162163
TEST_CASE("log zero arguments", "[mipi]") {

‎tools/CMakeLists.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
mypy_lint(FILES gen_str_catalog.py)
2+
3+
add_unit_test("gen_str_catalog_test" PYTEST FILES "gen_str_catalog_test.py")

0 commit comments

Comments
 (0)