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.

Sign up for GitHub

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

Jump to bottom

Reduce the stack usage in log function #2649

Merged
merged 5 commits into from
Oct 14, 2021
Merged

Reduce the stack usage in log function #2649

merged 5 commits into from
Oct 14, 2021
+43 −10

Conversation

xiaoxiang781216
Copy link
Collaborator

@xiaoxiang781216 xiaoxiang781216 commented Oct 9, 2021 •
edited
Loading

Description of the feature or fix

  • feat(printf): support %pV format specifier
  • fix(log): remove 768B temp buffer if LV_LOG_PRINTF == 1
  • fix(log): save 256B temp buffer if LV_LOG_PRINTF == 0

Checkpoints

@xiaoxiang781216 xiaoxiang781216 changed the title Reduce lv_log stack usage feat(printf): Support %pV format specifier Oct 9, 2021
@xiaoxiang781216 xiaoxiang781216 changed the title feat(printf): Support %pV format specifier feat(printf): support %pV format specifier Oct 10, 2021
@kisvegabor
Copy link
Member

Shouldn't lv_log.c be also updated?

@xiaoxiang781216
Copy link
Collaborator Author

Shouldn't lv_log.c be also updated?

Yes, it's in the next PR. I can't add it in this PR because MicroPython CI complain lv_format_t can't be found. I guess that CI can't support the dependent patches in one PR?

@kisvegabor
Copy link
Member

CI should handle if a problem is fixed in a new commit because it always runs on the last commit. Please, push it here to see what happens.

@xiaoxiang781216
Copy link
Collaborator Author

OK, let's what happen now.

@xiaoxiang781216
Copy link
Collaborator Author

Here is the error(https://github.com/lvgl/lvgl/pull/2649/checks?check_run_id=3859062425):

Error: ../../lib/lv_bindings/lvgl/src/misc/lv_log.c:84:9: error: unknown type name ‘lv_vaformat_t’
   84 |         lv_vaformat_t vaf = {format, &args};
      |         ^~~~~~~~~~~~~

@kisvegabor
Copy link
Member

It fails because the Micropython binding comes with LV_SPRINTF_CUSTOM enabled and lv_vaformat_t is typedfed in a #if LV_SPRINTF_CUSTOM == 0 block.

So lv_log.c should support standard sprintf too.

@xiaoxiang781216
Copy link
Collaborator Author

Ok, look like I need find a better solution to support both case.

@xiaoxiang781216 xiaoxiang781216 changed the title feat(printf): support %pV format specifier Reduce the stack usage in log function Oct 13, 2021
@xiaoxiang781216 xiaoxiang781216 force-pushed the va branch 3 times, most recently from 7717650 to 0e5f3be Compare October 13, 2021 16:10
@xiaoxiang781216
Copy link
Collaborator Author

@kisvegabor @embeddedt could you review this PR which:

  • avoid call fwrite
  • try to reduce the stack usage in various case

kisvegabor
kisvegabor reviewed Oct 14, 2021
View reviewed changes
@kisvegabor
Copy link
Member

Merging, thanks!

@kisvegabor kisvegabor merged commit b141636 into lvgl:master Oct 14, 2021
@xiaoxiang781216 xiaoxiang781216 deleted the va branch October 17, 2021 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@embeddedt embeddedt

@kisvegabor kisvegabor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@xiaoxiang781216 @kisvegabor