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

Added heltec wifi lora v3 board #668

Merged
merged 9 commits into from
Nov 23, 2024
Merged

Conversation

BigDurl
Copy link
Contributor

@BigDurl BigDurl commented Nov 20, 2024

Description

Added new dev_heltec_wifi_lora_v3 device and helteclorav3demo environment.
Changed build flag for dev_heltc_wifi_v3 from -DWIFI_Kit_32=1 to -DWIFI_Kit_32_V3.
Both flags do the same thing but passing the later may help compatibility in the future.
Fixed line in screen.h to disable LoRa by default.

Contributing requirements

  • I read the contribution guidelines in CONTRIBUTING.md.
  • I understand the BlinkenPerBit metric, and maximized it in this PR.
  • I selected main as the target branch.
  • All code herein is subjected to the license terms in COPYING.txt.

Added demo env for heltec wifi lora v3
Is messy and kind of hacked together
Set the correct pins for the OLED screen on the Heltec Wifi Lora V3 Board
Change backColor from 0,0,64 to 0,0,0 when using Heltec Wifi Lora V3 for compatibility with built-in OLED
Copy link
Collaborator

@rbergen rbergen left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this. I have a couple of questions and (for now) one change request.

Can you confirm that with these changes applied, you do have a working Heltec Wifi Lora V3 board, including display?

platformio.ini Outdated Show resolved Hide resolved
include/screen.h Outdated Show resolved Hide resolved
platformio.ini Outdated Show resolved Hide resolved
@BigDurl
Copy link
Contributor Author

BigDurl commented Nov 21, 2024

Yes, everything is working now.

Copy link
Collaborator

@rbergen rbergen left a comment

Choose a reason for hiding this comment

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

Thanks for your responses and the commit.

Based on one of your answers I think one more change is required to make sure we retain the current behavior on non-LoRa boards.

It definitely looks to me like we're getting there!

include/screen.h Outdated Show resolved Hide resolved
platformio.ini Outdated Show resolved Hide resolved
@rbergen
Copy link
Collaborator

rbergen commented Nov 23, 2024

@BigDurl I've made, committed and pushed the following changes:

  • I removed the heltec.h board defines (WIFI_*_32_V3) from the dev blocks in platformio.ini. It turns out they are already included in the board configurations provided by PlatformIO, so including them in platformio.ini yielded a heap of macro redefinition warnings.
  • I moved the setting of USE_SSD1306 to the dev blocks, in line with what I said before. What type of screen is connected is a board/device trait, not a project feature.
  • I converted the SCREEN_ROTATION define to a ROTATE_SCREEN feature flag, that makes the code in screen.h rotate the screen 180 degrees when set to 1. I've set that flag to 1 for the heltecv3demo env in platformio.ini as you suggested.

I think that with this, Heltec WiFi Kit/LoRa V3 defines are where they belong, and structured in line with the rest of NightDriverStrip.

Now that it seems that CI is happy with the PR as it stands, please have a look to see if you can also agree with the changes I made. If so, I'll merge.

@BigDurl
Copy link
Contributor Author

BigDurl commented Nov 23, 2024

Everything looks good.
One thing we might have to address eventually is. Currently we can only easily set the screen rotation to 0 or 180 and
I'm sure there's going to be cases where someone will want their display at 90 and 270.

@rbergen
Copy link
Collaborator

rbergen commented Nov 23, 2024

That's a fair comment, but if we do run into this then we will have to address another (and probably bigger) issue as well: the on-device displays are never square, so a simple non-180-degree rotation won't work anyway. Which is why I was comfortable with doing it this way, at least for now.

@rbergen rbergen merged commit 23ad510 into PlummersSoftwareLLC:main Nov 23, 2024
43 checks passed
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.

2 participants