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

ADD: heltec wsl v3 pinout information #7883

Merged
merged 3 commits into from
Apr 4, 2023
Merged

Conversation

gguerreromx
Copy link
Contributor

Added the required pin out file for Heltec Wireless Stick Lite v3 (Board from November 2022).

Information gathered from the site:
https://heltec.org/project/wireless-stick-lite-v2/ (it shares url with v2)

And Esp32 s3 technical document when required (SPI/I2C pins).

Information based on website and s3 manual
@CLAassistant
Copy link

CLAassistant commented Feb 23, 2023

CLA assistant check
All committers have signed the CLA.


#include <stdint.h>

#define Wireless_Stick_Lite true
Copy link
Contributor

@platypii platypii Feb 23, 2023

Choose a reason for hiding this comment

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

Please use a different macro name so we can distinguish this from V1. I would suggest
#define Wireless_Stick_Lite_V3 true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To my understanding, four main variants of the esp32/lora heltec boards exist, with I believe, three revisions each. While I understand the reasoning behind placing the version in the definition, this attribute is mainly used with their library (https://github.com/HelTecAutomation/Heltec_ESP32).

This definition allows the library to know what features are available in the board (big screen, small screen, no screen), allow external voltage, etc. (See code: https://github.com/HelTecAutomation/Heltec_ESP32/blob/master/src/heltec.cpp).

Since there is no "board hardware difference" between versions 1,2,3, placing this definition allows the library to work as expected and without depending on that library to update (since it hasn't updated for some revisions).

Copy link
Contributor

Choose a reason for hiding this comment

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

The Wireless_Stick_Lite and the Wireless_Stick_Lite_V3 are extremely different board hardware. The only thing similar is the form factor. The V3 uses a new ESP32-S3 cpu, different lora module, totally different pin numbers, and even different numbers of pins.

I understand you're trying to make this work with the heltec library, but please don't use a duplicate board macro here just to make an external library work. I have a PR open in that repo to support newer boards. You can also fork that repo and use your own version of the heltec library. Until they merge my PR there, I've been pointing PlatformIO at my own fork:

lib_deps = https://github.com/platypii/Heltec_ESP32.git

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do agree in your point since editing the repo is the actual solution and not doing it half way as I was attempting.

I just saw you PR and I believe it is a great idea to actually separate the features from the boards. Solves current issues and make it easier to add new boards in the future.

I hope it is accepted promptly.

Comment on lines +49 to +55
static const uint8_t T0 = 1;
static const uint8_t T1 = 2;
static const uint8_t T2 = 3;
static const uint8_t T3 = 4;
static const uint8_t T4 = 5;
static const uint8_t T5 = 6;
static const uint8_t T6 = 7;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't placed them since they do not exist as an available pin in the board itself (https://resource.heltec.cn/download/Wireless_Stick_Lite_V3/HTIT-WSL_V3.png).

They can be placed if required?

Should I place them?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a totally fair point. I added all of them because that's how it was here. I kind of agree with your point that we should only include pins which are actually available. But all the other heltec v3 pin files include all of them. Either way seems fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am hardly a tinkerer when it comes to electronics and for that reason I find it easier to think about what i can create/modify if there is a correlation between what I see in hardware and in code.

That is why I believe showing only available pins is the right way to go.

If there is no issue I would like to be kept as is.

In the other it didn't make much sense my logic of separating them by ADC#_ and Ax and A1x so I did fix the numeration to be continuous.

Comment on lines 33 to 47
static const uint8_t A0 = 1;
static const uint8_t A1 = 2;
static const uint8_t A2 = 3;
static const uint8_t A3 = 4;
static const uint8_t A4 = 5;
static const uint8_t A5 = 6;
static const uint8_t A6 = 7;
static const uint8_t A10 = 12;
static const uint8_t A12 = 14;
static const uint8_t A13 = 15;
static const uint8_t A14 = 16;
static const uint8_t A15 = 17;
static const uint8_t A16 = 18;
static const uint8_t A17 = 19;
static const uint8_t A18 = 20;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above about which pins to include. Either way works.

However, why do your GPIO pins change from being A+1 to A+2 starting at A10? That doesn't seem right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, it didn't make sense (see above)

variants/heltec_wireless_stick_lite_v3/pins_arduino.h Outdated Show resolved Hide resolved

#define EXTERNAL_NUM_INTERRUPTS 16
#define NUM_DIGITAL_PINS 40
#define NUM_ANALOG_INPUTS 15
Copy link
Contributor

Choose a reason for hiding this comment

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

NUM_ANALOG_INPUTS should be 20 for ESP32-S3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

ADD: proper versioning to the definition (added v3)
FIX: TX/RX inversion.
UPD: analog pins to be defined as a consecutive numeration.
FIX: number of digital pins.
@P-R-O-C-H-Y P-R-O-C-H-Y added Status: Pending Merge Pull Request is ready to be merged and removed Status: Review needed Issue or PR is awaiting review labels Apr 3, 2023
@me-no-dev me-no-dev merged commit bfe0306 into espressif:master Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Pending Merge Pull Request is ready to be merged Type: 3rd party Boards
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants