Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature test usart into release #12

Merged
merged 6 commits into from May 3, 2019

Conversation

Projects
None yet
2 participants
@PatrickDekker98
Copy link
Contributor

commented Apr 29, 2019

No description provided.

PatrickDekker98 added some commits Apr 18, 2019

Merge pull request #8 from R2D2-2019/release
pull release into master, feature test_usart was added
@@ -9,10 +9,12 @@
#############################################################################

# source files in this project (main.cpp is automatically assumed)
SOURCES := $(wildcard ../code/src/*.cpp)
#SOURCES := $(wildcard ../code/src/*.cpp)

This comment has been minimized.

Copy link
@itzandroidtab

itzandroidtab May 2, 2019

Contributor

comments?

This comment has been minimized.

Copy link
@PatrickDekker98

PatrickDekker98 May 3, 2019

Author Contributor

gone by 85fdce9



public:

test_usart_c(unsigned int baudrate, uart_ports_c usart_port);
test_usart_c();

/// @brief does not actualy disable anything

This comment has been minimized.

Copy link
@itzandroidtab

itzandroidtab May 2, 2019

Contributor

this comment is wrong btw. The function name is enable

This comment has been minimized.

Copy link
@PatrickDekker98

PatrickDekker98 May 3, 2019

Author Contributor

edited in 85fdce9

@@ -47,5 +48,17 @@ namespace r2d2 {
///@return unsigned int always 1

This comment has been minimized.

Copy link
@itzandroidtab

itzandroidtab May 2, 2019

Contributor

this doesn't always return 1 anymore

This comment has been minimized.

Copy link
@PatrickDekker98

PatrickDekker98 May 3, 2019

Author Contributor

edited in 85fdce9

}
}

void test_usart_c::set_receive_byte(const uint8_t byte){

This comment has been minimized.

Copy link
@itzandroidtab

itzandroidtab May 2, 2019

Contributor

this function sounds like the byte i set here will be the only byte that will be in the receive_buffer. Maybe a name change to something like add_receive_byte() is better here

This comment has been minimized.

Copy link
@itzandroidtab

itzandroidtab May 2, 2019

Contributor

or a clear before the push is oke as well i guess.

This comment has been minimized.

Copy link
@PatrickDekker98

PatrickDekker98 May 3, 2019

Author Contributor

was changed in 85fdce9

return receive_buffer.copy_and_pop();
}

bool test_usart_c::char_available() {

This comment has been minimized.

Copy link
@itzandroidtab

itzandroidtab May 2, 2019

Contributor

What is the difference between char_available() and available()? seems a bit redundant they return the exact same atm

This comment has been minimized.

Copy link
@PatrickDekker98

PatrickDekker98 May 3, 2019

Author Contributor

char_available returns true if char is available, available returns the amound of bytes available

This comment has been minimized.

Copy link
@itzandroidtab

itzandroidtab May 3, 2019

Contributor

But both char_available() and available() return !receive_buffer.empty() that is unintended then i guess

#include <test_usart.hpp>
#include <uart_ports.hpp>

TEST_CASE("test_usart_c sends","[test_usart_c]"){

This comment has been minimized.

Copy link
@itzandroidtab

itzandroidtab May 2, 2019

Contributor

maybe a test with the set_receive_string() and the set_receive_bytes()?

This comment has been minimized.

Copy link
@PatrickDekker98

PatrickDekker98 May 3, 2019

Author Contributor

were added in 85fdce9

@itzandroidtab
Copy link
Contributor

left a comment

Looks pretty nice to test the usart bus with actual data. Got some small points above.

@PatrickDekker98 PatrickDekker98 requested a review from itzandroidtab May 3, 2019

@itzandroidtab
Copy link
Contributor

left a comment

Looks good except the char_available() and the available() both return !receive_buffer.empty() and your comment suggests that available() should return a amount of data left

@PatrickDekker98 PatrickDekker98 requested a review from itzandroidtab May 3, 2019

@PatrickDekker98 PatrickDekker98 merged commit a3470c1 into release May 3, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.