Skip to content

tests: optimize xtimer drift for size#6662

Merged
jnohlgard merged 2 commits intoRIOT-OS:masterfrom
smlng:pr/tests/opt_xtimer_drift
Mar 9, 2017
Merged

tests: optimize xtimer drift for size#6662
jnohlgard merged 2 commits intoRIOT-OS:masterfrom
smlng:pr/tests/opt_xtimer_drift

Conversation

@smlng
Copy link
Copy Markdown
Member

@smlng smlng commented Feb 27, 2017

the test description was inlined with puts, which caused unnecessary blown up memory sizes.

This PR moves most of the text into a separate README. With this improvements the test works on arduino duemilanove with 2K RAM.

@smlng smlng added Platform: AVR Platform: This PR/issue effects AVR-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: tests Area: tests and testing framework labels Feb 27, 2017
@smlng smlng added this to the Release 2017.04 milestone Feb 27, 2017
@smlng smlng self-assigned this Feb 27, 2017
@smlng smlng requested a review from PeterKietzmann February 27, 2017 16:06
- remove descriptive puts messages, and use LOG_DEBUG/INFO
- rational: fits into 2k RAM now, and works on arduino duemilanove
@smlng smlng force-pushed the pr/tests/opt_xtimer_drift branch from 7846d25 to ae5ee83 Compare February 27, 2017 17:06
@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 1, 2017

If you optimize for size, maybe have a look if BOARD_INSUFFICIENT_MEMORY is still valid ;-)

@smlng
Copy link
Copy Markdown
Member Author

smlng commented Mar 1, 2017

@miri64 it wasn't in the first place: without this PR it wouldn't fit onto a arduino-duemilanove thats why I started this 😉 From the specs it should work with the blacklisted nucleos (BOARD_INSUFFICIENT_MEMORY := nucleo32-f042 nucleo32-f031) now, too. Unfortunately, as with many test cases: I do not have those boards at my disposal 😞

@smlng
Copy link
Copy Markdown
Member Author

smlng commented Mar 1, 2017

Besides: I think its better to have a README for tests, anyway - while the test apps in itself should be small without unnecessary bloat.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 1, 2017

Unfortunately, as with many test cases: I do not have those boards at my disposal 😞

You don't need it: assuming the board provider configured the linker correctly, just running the linker (i.e. run make all) would suffice. If it does not fit it will error.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Mar 1, 2017

Besides: I think its better to have a README for tests, anyway - while the test apps in itself should be small without unnecessary bloat.

I agree. Just want to keep the build tests up to date ;-)

@smlng
Copy link
Copy Markdown
Member Author

smlng commented Mar 1, 2017

@miri64 the nucleos still don't fit. At least not with the current board/cpu configuration, i.e., the problem is that even those small mem nucleos share common config from cpu/cortrexm_common where #define THREAD_STACKSIZE_DEFAULT (1024) is set, which is way to large here.

I created an issue see #6669.

@kYc0o kYc0o added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 3, 2017
@smlng
Copy link
Copy Markdown
Member Author

smlng commented Mar 9, 2017

ping!

Copy link
Copy Markdown
Member

@jnohlgard jnohlgard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! ACK

@jnohlgard jnohlgard merged commit d559428 into RIOT-OS:master Mar 9, 2017
@smlng smlng deleted the pr/tests/opt_xtimer_drift branch March 9, 2017 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: AVR Platform: This PR/issue effects AVR-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants