Skip to content

Removed ProS2 and added TinyS2 to boards.txt #5037

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

Merged
merged 4 commits into from
Apr 15, 2021
Merged

Removed ProS2 and added TinyS2 to boards.txt #5037

merged 4 commits into from
Apr 15, 2021

Conversation

UnexpectedMaker
Copy link
Contributor

I wont be shipping my ProS2 and instead am shipping my TinyS2.

@lbernstone
Copy link
Contributor

Looks like a nice board 🍾

@UnexpectedMaker
Copy link
Contributor Author

Looks like a nice board 🍾

Cheers :)

This is why my APA doesn't work on my FeatherS2 as it usess IO45 and the check is for < not <=
@lbernstone
Copy link
Contributor

lbernstone commented Apr 13, 2021

Modifying hal code is outside of the scope of a variant change, and I don't think you are correct. pin46 is input only (so NUM_OUTPUT_PINS really is 46).

@UnexpectedMaker
Copy link
Contributor Author

Modifying hal code is outside of the scope of a variant change, and I don't think you are correct. pin46 is input only (so NUM_OUTPUT_PINS really is 46).

This is NOT a variant change. This is specific to all S2 and S3 chips. Either way that number is wrong. It should at least be 46 as having it as 45 excludes IO45 which is a JTAG pin, so this is a bug that needs to be fixed.

@chegewara
Copy link
Contributor

It all depends what you mean by NUM_OUPUT_PINS. Is it highest number of the output pin or total number of output pins. But actually in both cases it is wrong value.

  • highest number of output pins is 45, pin IO46 is IN only,
  • total number of OUT pins is 42, because pin IO46 is IN only and there is no IO22-25 on S2 (why?)

Am i missing something?

@UnexpectedMaker
Copy link
Contributor Author

It all depends what you mean by NUM_OUPUT_PINS. Is it highest number of the output pin or total number of output pins. But actually in both cases it is wrong value.

  • highest number of output pins is 45, pin IO46 is IN only,
  • total number of OUT pins is 42, because pin IO46 is IN only and there is no IO22-25 on S2 (why?)

Am i missing something?

Yeah, it's an ambiguous name for sure, but in code it limits the MAX GPIO value that can be used... and that's just incorrect, as it prevents anyone from using IO45 in their Arduino code.

@chegewara
Copy link
Contributor

chegewara commented Apr 14, 2021

@UnexpectedMaker
Copy link
Contributor Author

I would rather propose to fix this line:
https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-gpio.c#L212

Is that the only place it's used?
I don't mind how it's fixed, just that it gets fixed (and quickly) as blocking this IO is nasty - especially for my FeatherS2 :)

@chegewara
Copy link
Contributor

It is also used in this code:
https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/esp32-hal-gpio.c#L255-L270

So, in that case your suggestion of changing it in define is good, but value shall be 46 not 47.

@me-no-dev me-no-dev merged commit ec7aeb4 into espressif:master Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants