8000 Fix isRaspian() to properly detect Buster image by gerth2 · Pull Request #637 · PhotonVision/photonvision · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix isRaspian() to properly detect Buster image #637

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
Merged
merged 15 commits into from
Dec 27, 2022
Merged

Fix isRaspian() to properly detect Buster image #637

merged 15 commits into from
Dec 27, 2022

Conversation

gerth2
Copy link
Contributor
@gerth2 gerth2 commented Dec 18, 2022

Revised isRaspian() call to look in multiple spots to check if we're a Pi or not

@gerth2 gerth2 requested a review from a team as a code owner December 18, 2022 00:24
@gerth2
Copy link
Contributor Author
gerth2 commented Dec 18, 2022

Whelp this snowballed. That's ok. Here's today's direction:

  1. Un-intertwine Platform and PiVersion
  2. make the actual platform enum value private
  3. Constrain Platform's public API down to is*() boolean getters which are descriptive of what the use case actually is
  4. rework users of Platform to use the correct public API, which will change functionality slightly for supporting new miniPC's or SBC's

So far, the public API is down to just:

isRoot() - whether or not certain commands can be expected to succeed
unixSupported() - whether or not we expect a unix shell and unix-API queues to work
isRaspberryPi() - whether or not we expect Pi-specific SBC hardware to be present (GPU accel, GPIO, etc.)

TODO still: Rework the metrics command to use the hardcoded values for Pi's, but still support and run on non-Pi platform IFF the user supplies the approprate shell commands.

@prensing
Copy link
Contributor

You are going to run into some problems on a Mac. You defined "unixSupported" as Mac or Linux, and that is fine. But you are using unixSupported in a lot of places where you really should be testing for Linux. For example, in the code to restart the photonvision executable, "systemctl" is very definitely only on Linux (and then not even on some).

@gerth2
Condensed metrics classes, but expanded the configuration to default …
248f873 
…to file, but fall back on hardcoded commands for certain platforms
@gerth2
Merge remote-tracking branch 'origin/master' into metrics_not_updating
39dd328 
# Conflicts:
#	photon-core/src/main/java/org/photonvision/common/hardware/Platform.java
#	photon-core/src/main/java/org/photonvision/common/networking/NetworkManager.java
@gerth2
Migrate unixSupported to isLinux
3a41f5a
@gerth2
applied spotless
9906f75
@gerth2
wpiformat
e28ef7e
@prensing
Copy link
Contributor

I created a PR to merge some changes into this branch. It implements cmds for a general Linux platform.

@mdurrani808
Copy link
Contributor

Closes #637

mdurrani808 and others added 2 commits December 24, 2022 21:44
@mdurrani808
Merge branch 'master' into metrics_not_updating
6530cff
@prensing
Linux metrics (#641)
e33a59a 
* Move generic commands from PiCmds to LinuxCmds; have PiCmds inherit from LinuxCmds

* Better names for variables to save the total memory values

* Remove "Bionic" from the architecture; that is not actually determined.

* Trigger PhotonVision CI

* Dummy change to trigger CI
@mdurrani808 mdurrani808 mentioned this pull request Dec 25, 2022
Closed
@mcm001
Copy link
Contributor
mcm001 commented Dec 27, 2022

Works on my pi 3 at least Dec 26 21:44:05 photonvision java[5854]: [2022-12-26 21:44:05] [General - Main] [INFO] Starting PhotonVision version dev-v2023.1.1-beta-6-24-ge33a59a2 on Linux Raspbian 64-bit (Pi PI_3)

@mcm001
Run format
83c5d34 
Update index.html
mcm001
mcm001 previously approved these changes Dec 27, 2022
View reviewed changes
@mcm001 mcm001 merged commit b190595 into master Dec 27, 2022
@mcm001 mcm001 deleted the metrics_not_updating branch December 27, 2022 03:52
MrRedness pushed a commit to MrRedness/photonvision that referenced this pull request Feb 8, 2023
@gerth2 @mdurrani808
@prensing @mcm001 @MrRedness
Fix isRaspian() to properly detect Buster image (PhotonVision#637)
e94e52a 
* Revised isRaspian() call to look in multiple spots to check if we're a Pi or not

* wpiformat

* linefeed fixup

* whoops

* WIP updating platform

* More platform fixups WIP

* Condensed metrics classes, but expanded the configuration to default to file, but fall back on hardcoded commands for certain platforms

* Migrate unixSupported to isLinux

* applied spotless

* wpiformat

* Linux metrics (PhotonVision#641)

* Move generic commands from PiCmds to LinuxCmds; have PiCmds inherit from LinuxCmds

* Better names for variables to save the total memory values

* Remove "Bionic" from the architecture; that is not actually determined.

* Trigger PhotonVision CI

* Dummy change to trigger CI

* Run format

Update index.html

Co-authored-by: Mohammad Durrani <46766905+mdurrani808@users.noreply.github.com>
Co-authored-by: Paul Rensing <prensing@gmail.com>
Co-authored-by: Matt <matthew.morley.ca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcm001 mcm001 mcm001 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gerth2 @prensing @mdurrani808 @mcm001
0