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

[io] Refactor IOStream to use an external printf implementation #199

Merged
merged 2 commits into from
Apr 28, 2019

Conversation

salkinium
Copy link
Member

@salkinium salkinium commented Apr 27, 2019

Our printf implementation was good for many years, however, a lot of features were missing and our tests, while helpful, weren't complete.
In addition I wanted to refactor IOStream for a long time now, and add some feature options for the AVRs to selectively enable long long, float, ptrdiff and printf-style formatting.

This completely replaces all formatting code with mpaland/printf, and cleans up the IOStream class, with only these changes to the behavior:

  • %p does not print the leading 0x anymore, you have to add it yourself.
  • printing bools in binary mode only prints 0 or 1 not the leading 7 zeros anymore. stream << modm::bin << (bool)1 prints 1, not 00000001.
  • passing a const char * array will always print in ascii mode. stream << modm::hex << "Hallo" prints Hallo, not 48616C6C6F00.
  • For AVRs any options for %f, %e or %g are ignored and the value is output in scientific notation.

Ran unittests on:

  • hosted-darwin
  • STM32
  • AVR

cc @se-bi @rleh @chris-durand @strongly-typed

@salkinium
Copy link
Member Author

I was concerned about AVR code size, so I did a regression test and for using the streaming interface it looks pretty much the same, but printf on the other hand has gotten bigger when compiled with long long support and floating point support. I think this is fine, since the new printf contains a lot better formatting support, so this increase is acceptable.

AVR Baseline:

#include <modm/platform.hpp>
#include <modm/io.hpp>

using LoggerDevice = modm::IODeviceWrapper< modm::platform::Uart0, modm::IOBuffer::BlockIfFull >;
LoggerDevice loggerDevice;
modm::IOStream ostream(loggerDevice);

int
main()
{
	// test code
}

840 bytes

Output stream only:

{{type}} value = 0;
ostream << value;
type before after :io:with_long_long :io:with_float
bool 1130 1196
char 1030 1026
uint8_t 1172 1176
int16_t 1246 1216
uint16_t 1188 1192
int32_t 1284 1244
uint32_t 1256 1216
int64_t n/a 2682 x
uint64_t n/a 2542 x
float 2350 2526 x
double 2350 2526 x
ostream 1808 1756
ostream n/a 3662 x
ostream 3342 3344 x
ostream n/a 5248 x x

printf only:

{{type}} value = 0;
ostream.printf("%{{type}}", value);;
type before after :io:with_long_long :io:with_float
printf 3522 3390
printf n/a 4636 x
printf 3522 5074 x
printf n/a 6302 x x

@salkinium salkinium merged commit 4ce1a47 into modm-io:develop Apr 28, 2019
@salkinium salkinium deleted the feature/external_printf branch July 23, 2019 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

1 participant