Skip to content

Remove LED and GPIO support #2065

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Remove LED and GPIO support #2065

wants to merge 2 commits into from

Conversation

samfreund
Copy link
Member

@samfreund samfreund commented Aug 15, 2025 •
edited
Loading

Description

LEDs are no longer used with retro-reflective being removed. GPIO was added for LED support, and with the removal of LEDs is no longer needed.

closes #2062

Meta

Merge checklist:

  • Pull Request title is short, imperative summary of proposed changes
  • The description documents the what and why
  • If this PR changes behavior or adds a feature, user documentation is updated
  • If this PR touches photon-serde, all messages have been regenerated and hashes have not changed unexpectedly
  • If this PR touches configuration, this is backwards compatible with settings back to v2024.3.1
  • If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated
  • If this PR addresses a bug, a regression test for it is added

@samfreund samfreund requested a review from a team as a code owner August 15, 2025 20:15
@github-actions github-actions bot added documentation Anything relating to https://docs.photonvision.org frontend Having to do with PhotonClient and its related items photonlib Things related to the PhotonVision library backend Things relating to photon-core and photon-server labels Aug 15, 2025
@samfreund samfreund force-pushed the rip-gpio branch 3 times, most recently from 6df8be9 to a2de9f0 Compare August 16, 2025 00:35
@samfreund samfreund requested a review from Gold856 August 16, 2025 00:37
Gold856
Gold856 requested changes Aug 16, 2025
View reviewed changes
@samfreund samfreund force-pushed the rip-gpio branch 2 times, most recently from 6892b8b to 4c5e09f Compare August 16, 2025 06:10
@samfreund samfreund requested a review from Gold856 August 16, 2025 06:11
@crschardt
Copy link
Contributor

Should we also remove pigpio and the associated config files from our RPi images? https://github.com/PhotonVision/photon-image-modifier/blob/1611214fea86b22fafb2ddc1a0f5ab4f29f65e72/install_pi.sh#L33

@samfreund samfreund force-pushed the rip-gpio branch 2 times, most recently from fe9c6be to 671ee8f Compare August 20, 2025 01:49
@samfreund samfreund mentioned this pull request Aug 20, 2025
Closed
Gold856

This comment was marked as resolved.

Sign in to view

@samfreund
Copy link
Member Author

Remove LightingSettings.

done

Gold856
Gold856 requested changes Aug 20, 2025
View reviewed changes
@samfreund samfreund requested a review from Gold856 August 20, 2025 05:41
spacey-sooty
spacey-sooty reviewed Aug 20, 2025
View reviewed changes
Copy link
Member

@spacey-sooty spacey-sooty left a comment

Choose a reason for hiding this comment

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

I'm inclined to keep a GPIO library in the image so people don't break the image if they try and do LED control for their own purposes

docs/source/docs/additional-resources/config.md
@@ -22,8 +22,6 @@ The directory structure is outlined below.
- Contains images saved with the input/output save commands.
- logs
- Contains timestamped logs in the format {code}`photonvision-YYYY-MM-D_HH-MM-SS.log`. These timestamps will likely be significantly behind the real time. Coprocessors on the robot have no way to get current time.
- hardwareSettings.json
Copy link
Member

Choose a reason for hiding this comment

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

Why not delete the hardwareConfig.json then?

Copy link
Member Author

Choose a reason for hiding this comment

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

what's that do again?

Copy link
Member

Choose a reason for hiding this comment

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

As of right now just LEDs. If we added support for gyros (ie canandgyro over usb or LL4 gyro) it make sense to add it back but we can cross that bridge when we come to it

Copy link
Member Author

Choose a reason for hiding this comment

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

sure, yoink

Gold856
Gold856 previously approved these changes Aug 20, 2025
View reviewed changes
Copy link
Collaborator

@Gold856 Gold856 left a comment

Choose a reason for hiding this comment

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

As long as old configs still work with no errors, this looks great!

@gerth2
Copy link
Contributor

gerth2 commented Aug 21, 2025

What's the driving reason to remove the status LED's? Casserole used them two years in a row for custom hardware, and found it quite valuable for on-field debug.

@samfreund samfreund dismissed Gold856’s stale review August 22, 2025 05:47

We're keeping most of it for status LEDs, just dumping the stuff for lighting since retro-reflective is gone now.

@samfreund
Copy link
Member Author

We're going to keep GPIO and status LEDs, but get rid of lighting. Alongside that, we're going to change our GPIO lib to libgpiod.

@samfreund
Copy link
Member Author

Also @spacey-sooty do you know what pins the status lights for LLs are on?

mcm001
mcm001 reviewed Aug 31, 2025
View reviewed changes
...-core/src/main/java/org/photonvision/common/dataflow/networktables/NetworkTablesManager.java
@@ -158,7 +157,6 @@ public void checkNtConnectState(NetworkTableEvent event) {
event.connInfo.remote_port,
event.connInfo.protocol_version);
logger.error(msg);
HardwareManager.getInstance().setNTConnected(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

If the plan is to retain status LED support, revert this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcm001 mcm001 mcm001 left review comments

@spacey-sooty spacey-sooty spacey-sooty left review comments

@Gold856 Gold856 Gold856 left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
backend Things relating to photon-core and photon-server documentation Anything relating to https://docs.photonvision.org frontend Having to do with PhotonClient and its related items photonlib Things related to the PhotonVision library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Drop GPIO support
6 participants
@samfreund @crschardt @gerth2 @mcm001 @spacey-sooty @Gold856