-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
samples: compression: exclude esp32s3 board from testing #65081
samples: compression: exclude esp32s3 board from testing #65081
Conversation
esp32s3_devkitm_appcpu has not enough RAM memory to support this test. Therefore we can exclude this from testing. Signed-off-by: Sylvio Alves <sylvio.alves@espressif.com>
7469fb9
to
f1c00c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this exclusion be purely dealt with based on the min_ram
required by the sample? BTW is there a reason (I'm sure there is!) why xtensa boards don't set ram: and rom: in their yaml file?
In any case, I think there was a regression with PR #63332 and the malloc arena is probably way larger than it should. Can you try CONFIG_COMMON_LIBC_MALLOC_ARENA_SIZE=24576
? The sample seem to run just fine for me with this (qemu cortexm3 and native posix).
@keith-packard do you remember if there was a reason for setting such a large (64K) arena for the lz4 sample?
@kartben, in case of |
As per my comment, it looks like the sample requires actually way less than one might have thought. By changing the malloc arena size to 24K, it seems to fit just fine in the appcpu (haven't tried to run on an actual target though)
|
Sure, then it is up to verifying whether lz4 compression sample can work with that 24k arena size. If yes, this PR can be closed. Edit: Ok, you have tested with qemu and native posix. It is also ok on esp32s3 side as well. |
Superseded by #65129 |
esp32s3_devkitm_appcpu has not enough RAM memory to support this test. Therefore we can exclude this from testing.