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

Support for the ESP-IDF framework #144

Merged
merged 1 commit into from
Aug 6, 2023
Merged

Conversation

ivmarkov
Copy link
Contributor

@ivmarkov ivmarkov commented Aug 6, 2023

The TimerOp struct size - depending on the platform - is anything between 20 to 28 bytes (if not more due to alignment). When multiplied by 1000 timers for the bounded queue we get into 20K to 30K RAM usage.

Minuscule amount for regular machines or even for embedded-Linux SOCs, but quite a lot for MCUs which have 200 to 400K RAM.

Reducing the queue to 100 timers seems reasonable to me, because I don't expect hundreds of timers to be used on that platform.

Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

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

Could you also add ESP-IDF to the CI?

src/reactor.rs Outdated Show resolved Hide resolved
@ivmarkov ivmarkov force-pushed the espidf branch 2 times, most recently from 87a7bf5 to d34d1da Compare August 6, 2023 16:13
Comment on lines +48 to +51
# - name: Check selected Tier 3 targets
# if: startsWith(matrix.rust, 'nightly') && matrix.os == 'ubuntu-latest'
# run: cargo check -Z build-std --target=riscv32imc-esp-espidf
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See this: #144 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, thanks for the explanation. Could you add a todo comment with the explanation on it or a link to your comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@ivmarkov
Copy link
Contributor Author

ivmarkov commented Aug 6, 2023

Could you also add ESP-IDF to the CI?

I did, but for now the actual cargo check for ESP-IDF is commented out, because:

  • We need a new release of polling (obviously or else the build would fail, as ESP-IDF support is only in polling/master ATM). I can patch polling to master during the build, but then see below
  • We need branch notgull/polling-breaking-changes merged as well, or else async-io will not compile against latest polling

Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

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

Thanks!

@notgull notgull merged commit d9c9ed8 into smol-rs:master Aug 6, 2023
21 checks passed
@notgull notgull mentioned this pull request Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants