Unit test for Util

[raavilagoo]

1.EnumMap 2.EnumSet 3.OrderedMap 4.RingBuffer

[ethanjli]

Example of the overall structure for RingBuffer tests

newest: ^
oldest: *
Scenario (non-volatile ringbuffer)
Given a ringbuffer with capacity 5 to which 4 bytes of data were pushed and then 3 bytes of data was popped
// internal state should look like [] [] [*a] [^b] []
    When: pop
        // then internal state should look like [] [] [] [*^b] []
        Then: (peeks)
    When: push
        // then internal state should look like [] [] [] [*b] [^c]
Given: (... for [] [] [o] [] [n])
    When: pop
        Then: (... for [] [] [] [o] [n])

Scenario (volatile ringbuffer)
Given a volatile ringbuffer...

[raavilagoo]

Hi @ethanjli , I wanted some clarification with respect to constructing a vector with initializer list , I have declared vector as this, PF::Util::Containers::Vector<uint8_t, 4> vector1{10, 20, 30}. If I check the values at every index, I get 0. Could you elaborate on this? Similarly with EnumMap, OrderedMap and EnumSet.

[ethanjli]

I tried declaring some container instances at the top of the main function in main.cpp and inspecting them with the STM32 debugger. Here are my results

• PF::Util::Containers::Vector<uint8_t, 5> vec{0, 1, 2, 3, 4}; causes vec to be initialized with an internal buffer which is all zeroes. So I'm able to reproduce the problem you found with how I expected the initializer list to work vs. how it actually works.
• PF::Util::Containers::Vector<uint8_t, 5> vec({0, 1, 2, 3, 4}); has the same problem.
• std::initializer_list<uint8_t> init{0, 1, 2, 3, 4}; PF::Util::Containers::Vector<uint8_t, 5> vec(init); also has the same problem.
• When I use the debugger to step through the Vector constructor defined on the initializer list, what I see is that the first push_back method called in the constructor returns early with IndexStatus::out_of_boudns, because size_ == array_size. In this case, size_ was already initialized as 5, because the constructor pre-initialized size_ to 5. This is clearly a bug because I was careless in implementing the constructor and because I never tested my code.

By removing the size_(init.size()) member initialization from the Vector(std::initializer_list<Element> init) constructor, I was able to fix the bug and get the correct result for PF::Util::Containers::Vector<uint8_t, 5> vec{0, 1, 2, 3, 4};. Can you include this fix into this PR?

Also, we should make sure that you will have easy access to an STM32H743 Nucleo board so that you'll be able to upload the firmware with some test code added to the top of the main function, just to be able to take advantage of the debugger for troubleshooting these kinds of issues in lower-level code even without a fully working ventilator setup. @renjipanicker Is this already possible / how easy would it be to make this possible, so that both Rohan and Raavi have easy access to Nucleo boards for troubleshooting software-only issues using the STM32Cube IDE's debugger?

[raavilagoo]

Hi @ethanjli, this is ready for review.

[raavilagoo]

@ethanjli This is ready for review.

[ethanjli]

I believe there are a total of 25 cases to cover for a RingBuffer with buffer_size == 5 (i.e. a capacity of 4 elements).

Notation:

• * indicates the element addressed by oldest_index_
• ^ indicates the element addressed by newest_index_

There are four types of "cells"/elements in the list of cases:

• [* ^] represents oldest_index_ == newest_index_, i.e. the buffer is empty
• [*1 ] represents "the oldest element in the buffer is 1"
• [ n ] represents an element in the buffer whose value is n
• [ ^] represents the element at newest_index_, which never has a value (it's always left as an empty space)

So a RingBuffer with template parameter buffer_size can store a maximum of buffer_size - 1 elements. In either this PR or a separate one, we should consider changing the buffer_size template parameter to a capacity template parameter, where buffer_size == capacity + 1; to be clear, "capacity" means "the maximum number of elements we can actually store in the container". This would be more consistent with our other containers, which all use capacity as the template parameter whenever buffer_size != capacity.

Cases:

Group 0: oldest_index_ == 0
[* ^] [   ] [   ] [   ] [   ]: newest_index == 0
[*1 ] [  ^] [   ] [   ] [   ]: newest_index == 1
[*1 ] [ 2 ] [  ^] [   ] [   ]: newest_index == 2
[*1 ] [ 2 ] [ 3 ] [  ^] [   ]: newest_index == 3
[*1 ] [ 2 ] [ 3 ] [ 4 ] [  ^]: newest_index == 4
Group 1: oldest_index_ == 1
[   ] [* ^] [   ] [   ] [   ]: newest_index == 1
[   ] [*1 ] [  ^] [   ] [   ]: newest_index == 2
[   ] [*1 ] [ 2 ] [  ^] [   ]: newest_index == 3
[   ] [*1 ] [ 2 ] [ 3 ] [  ^]: newest_index == 4
[  ^] [*1 ] [ 2 ] [ 3 ] [ 4 ]: newest_index == 0
Group 2: oldest_index_ == 2
[   ] [   ] [* ^] [   ] [   ]: newest_index == 2
[   ] [   ] [*1 ] [  ^] [   ]: newest_index == 3
[   ] [   ] [*1 ] [ 2 ] [  ^]: newest_index == 4
[  ^] [   ] [*1 ] [ 2 ] [ 3 ]: newest_index == 0
[ 4 ] [  ^] [*1 ] [ 2 ] [ 3 ]: newest_index == 1
Group 3: oldest_index_ == 3
[   ] [   ] [   ] [* ^] [   ]: newest_index == 3
[   ] [   ] [   ] [*1 ] [  ^]: newest_index == 4
[  ^] [   ] [   ] [*1 ] [ 2 ]: newest_index == 0
[ 3 ] [  ^] [   ] [*1 ] [ 2 ]: newest_index == 1
[ 3 ] [ 4 ] [  ^] [*1 ] [ 2 ]: newest_index == 2
Group 4: oldest_index_ == 4
[   ] [   ] [   ] [   ] [* ^]: newest_index == 4
[  ^] [   ] [   ] [   ] [*1 ]: newest_index == 0
[ 2 ] [  ^] [   ] [   ] [*1 ]: newest_index == 1
[ 2 ] [ 3 ] [  ^] [   ] [*1 ]: newest_index == 2
[ 2 ] [ 3 ] [ 4 ] [  ^] [*1 ]: newest_index == 3

This way of listing cases might make it easier to organize the test code: first you set up the empty ring buffer with oldest_index initialized to the correct location from 0 to 4 (by repeatedly pushing and popping an element n times, where n ranges from 0 to 4), and then you simply push between 0 and 4 elements into the ring buffer. This should initialize your GIVEN.

If there's any possible way we can unify some of the test code into reusable, parameterized functions (e.g. making functions consisting of REQUIRE assertions, and then each THEN calls a function with a parameter; and/or making a function to create an empty ring buffer with oldest_index initialized to the desired value; and/or making a function to create any one of our 25 initial states for the ring buffer; and/or making a function to push some specified number of elements; etc.), we should do that for maintainability.

[raavilagoo]

For records-keeping:

This project is licensed under Apache License v2.0 for any software, and Solderpad Hardware License v2.1 for any hardware - do you agree that your contributions to this project will be under these licenses, too? - Yes Were any of these contributions also part of work you did for an employer or a client? - No Does this work include, or is it based on, any third-party work which you did not create? - No