Skip to content

Added xtensalx6 for ESP32/ESP8266 platforms - #7977

Merged
czoido merged 2 commits into
conan-io:developfrom
redradist:feature/add-xtensalx6-arch
Sep 29, 2021
Merged

Added xtensalx6 for ESP32/ESP8266 platforms#7977
czoido merged 2 commits into
conan-io:developfrom
redradist:feature/add-xtensalx6-arch

Conversation

@redradist

@redradist redradist commented Oct 31, 2020 •
edited by czoido
Loading

Copy link
Copy Markdown
Contributor

Changelog: Feature: New setttings: xtensalx6 and xtensalx106 for ESP32/ESP8266 platforms.
Docs: omit

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

There is related pull-request in cmake-conan repository conan-io/cmake-conan#293

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@CLAassistant

CLAassistant commented Oct 31, 2020 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

memsharded
memsharded reviewed Oct 31, 2020
View reviewed changes
Comment thread conans/client/conf/__init__.py Outdated
@czoido

czoido commented Nov 24, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hi @redradist,
I think adding xtensalx6 as architecture may not be enough to model this.
For what I have seen, I think that it would be better to add a separate new os, that maybe could be ESP and under it a subsetting like cpu that depending on the chip could be lx6 (ESP32), l106(ESP8266) and leave arch as just xtensa
I have also seen that are two different choices for the target that are elf or linux (https://gcc.gnu.org/install/specific.html#xtensa-x-elf). Maybe that could be another subsetting?

WDYT @memsharded, @SSE4 ?

@redradist

redradist commented Nov 24, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

ESP

@czoido But there is not such OS like ESP !!

ESP32 works on FreeRTOS and Zephyr

xtensalx6 is exact name of architecture according the Wiki and it is similar to armv7 and etc.

@czoido

czoido commented Nov 24, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

@czoido But there is not such OS like ESP !!

@redradist, I know there is not such OS, I was just wondering how that can be modelled, and took a model like the one we have for Arduino or Emscripten that are also not os's but yes, if those chips work on FreeRTOS and Zephyr then those os's are the ones that should be in os.
One question, isn't xtensalx6 just valid for ESP32? Shouldn't the one for ESP8266 be something like xtensalx106 ?
Could you please link that wiki?
Thanks

@redradist

redradist commented Nov 24, 2020

Copy link
Copy Markdown
Contributor Author

@czoido But there is not such OS like ESP !!

@redradist, I know there is not such OS, I was just wondering how that can be modelled, and took a model like the one we have for Arduino or Emscripten that are also not os's but yes, if those chips work on FreeRTOS and Zephyr then those os's are the ones that should be in os.
One question, isn't xtensalx6 just valid for ESP32? Shouldn't the one for ESP8266 be something like xtensalx106 ?
Could you please link that wiki?
Thanks

@czoido ESP32 https://en.wikipedia.org/wiki/ESP32, for gcc seems like it is called just xtensa

For ESP8266 in wiki nothing mentioned regarding the subset of xtensa architechure (:

@czoido

czoido commented Nov 25, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

I think then probably the best way of adding this is adding architectures: xtensa_lx6 for ESP32 and xtensa_lx106 for ESP8266 and also adding os FreeRTOS (is that the default os for these boards?)

@redradist

redradist commented Dec 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

xtensa_lx6

@memsharded @czoido Maybe better xtensalx6 and xtensalx106, because it would be similar with aarch64, arm64v8, aarch64, arm64, armv7 ...
What do you think ?

@redradist

redradist commented Apr 12, 2021

Copy link
Copy Markdown
Contributor Author

@memsharded @czoido Have you seen my comment above ?

@redradist
redradist force-pushed the feature/add-xtensalx6-arch branch 5 times, most recently from 965fae0 to 7052d4c Compare September 12, 2021 14:43
@redradist

redradist commented Sep 12, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@memsharded @czoido Have you seen my comment above ?

@madebr Could you also review this PR ?

It is only 4 times added xtensalx6 target arch

@redradist
redradist force-pushed the feature/add-xtensalx6-arch branch from 7052d4c to adf70a1 Compare September 12, 2021 14:56
madebr
madebr approved these changes Sep 12, 2021
View reviewed changes

@madebr madebr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

ESP32 -> Xtensa LX6 (https://en.wikipedia.org/wiki/ESP32)
ESP8266 -> Xtensa L106 (https://en.wikipedia.org/wiki/ESP8266)

Is there a check that ensures all settings of the previous settings were copied correctly?

@redradist

redradist commented Sep 12, 2021

Copy link
Copy Markdown
Contributor Author

@madebr

Is there a check that ensures all settings of the previous settings were copied correctly?

Hard to say ...

@czoido

czoido commented Sep 15, 2021

Copy link
Copy Markdown
Contributor

Hi @redradist,
Sorry for the delay, I'm adding this PR to milestone 1.41 to be considered.
Mentioning #4216 that is slightly related

@czoido czoido added this to the 1.41 milestone Sep 15, 2021
@redradist
redradist force-pushed the feature/add-xtensalx6-arch branch from caec5d8 to 967d04d Compare September 21, 2021 18:44
@redradist

redradist commented Sep 21, 2021

Copy link
Copy Markdown
Contributor Author

@madebr @memsharded This PR passed CI

Could you please review it ?

@czoido
czoido merged commit 276d471 into conan-io:develop Sep 29, 2021
@czoido

czoido commented Sep 29, 2021

Copy link
Copy Markdown
Contributor

This will be released in Conan 1.41

@redradist

redradist commented Sep 30, 2021

Copy link
Copy Markdown
Contributor Author

@czoido @memsharded Probably it would be nice to create the specific compiler for it xtensa-esp32-elf-gcc ??
Is there a guidance how to do it ?

@redradist redradist mentioned this pull request Apr 29, 2022
Closed
@b1ackviking b1ackviking mentioned this pull request May 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@czoido czoido czoido approved these changes
@memsharded memsharded Awaiting requested review from memsharded
+1 more reviewer
@madebr madebr madebr approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@czoido czoido

Labels

None yet

Projects

None yet

Milestone

1.41

Development

Successfully merging this pull request may close these issues.

5 participants

@redradist @CLAassistant @czoido @madebr @memsharded