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

Fix for issue #8368 #8377

Merged
merged 2 commits into from Oct 22, 2018

Conversation

Projects
None yet
6 participants
@mprse
Member

mprse commented Oct 11, 2018

Description

This is fix for issue #8368.

Test is causing some problems on REALTEK_RTL8195AM and ARM compiler. There is some kind of memory issue. Probably there is not enough memory space for global data provided by the test. Data definitions have been moved into test function body so, they will land on stack. With this fix the test works on REALTEK_RTL8195AM/ARM.

Additionally optimize the test by using templates instead of global variables.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

mprse added some commits Oct 11, 2018

tests-mbed_hal-rtc_time: alocate required test data on stack
This is fix for issue 8368.

Test is causing some problems on `REALTEK_RTL8195AM` and `ARM` compiler. There is some kind of memory issue. Probably there is not enough memory space for global data provided by the test. Data definitions have been moved into test function body so, they will land on stack. With this fix the test works on `REALTEK_RTL8195AM/ARM`.
@studavekar

This comment has been minimized.

Collaborator

studavekar commented Oct 12, 2018

@mprse thanks for the fix 👍

@mprse

This comment has been minimized.

Member

mprse commented Oct 12, 2018

@M-ichae-l
I made some further tests and found that on REALTEK_RTL8195AM and AMR compiler I can declare in the test up to 90 bytes of initialised global data (located in .data section). For example on GCC_ARM I am able to allocate 10k (couldn't check on IAR). Could you check that, maybe there is some problem is scatter file?

Here is the test code:

#include "utest/utest.h"
#include "unity/unity.h"
#include "greentea-client/test_env.h"

#include "mbed.h"
#include "mbed_mktime.h"

char buffer[1024*10] = {0xff};

using namespace utest::v1;

void test()
{
    memset(&buffer,0, sizeof(buffer));
}

Case cases[] = {
    Case("test", test)
};

utest::v1::status_t greentea_test_setup(const size_t number_of_cases)
{
    GREENTEA_SETUP(20, "default_auto");
    return greentea_test_setup_handler(number_of_cases);
}

Specification specification(greentea_test_setup, cases, greentea_test_teardown_handler);

int main()
{
    return Harness::run(specification);
}
@studavekar

This comment has been minimized.

Collaborator

studavekar commented Oct 17, 2018

@@ -53,6 +51,7 @@ bool is_leap_year(int year)
* When _rtc_is_leap_year() function is called.
* Then _rtc_is_leap_year() returns true if given year is a leap year; false otherwise.
*/
template <rtc_leap_year_support_t rtc_leap_year_support>

This comment has been minimized.

@cmonr

cmonr Oct 17, 2018

Contributor

Why is this defined twice?

This comment has been minimized.

@mprse

mprse Oct 18, 2018

Member

Sorry, but what is defined twice?

This comment has been minimized.

@cmonr

cmonr Oct 18, 2018

Contributor

Lines 54 and 93

This comment has been minimized.

@mprse

mprse Oct 19, 2018

Member

Because these are two different template functions. Is it possible to define one template for many functions? I have never seen such case.

This comment has been minimized.

@cmonr

cmonr Oct 19, 2018

Contributor

It just seemed odd to me, but our local C++ expert (@geky) mentioend that it's par for the course. I'm use to seeing templates be used in classes, not functions.

@cmonr

Iiinteresting. Have a question, but should be good.

@cmonr

cmonr approved these changes Oct 19, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Oct 21, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 21, 2018

Build : SUCCESS

Build number : 3414
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8377/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr cmonr merged commit 1123c2b into ARMmbed:master Oct 22, 2018

15 checks passed

ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed , RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Passed, 655 files (+0 files)
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9942 cycles (+814 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8372B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@cmonr cmonr removed the ready for merge label Oct 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment