Skip to content

os: Enable GPS on adafruithat only#591

Merged
sonnyp merged 2 commits into
masterfrom
no-gps-overclocking-on-planktoscope-hat
Apr 30, 2025
Merged

os: Enable GPS on adafruithat only#591
sonnyp merged 2 commits into
masterfrom
no-gps-overclocking-on-planktoscope-hat

Conversation

@sonnyp
Copy link
Copy Markdown
Collaborator

@sonnyp sonnyp commented Apr 30, 2025

The GPS hat is not tested and probably incompatible with PlanktoScope hat.

@sonnyp sonnyp merged commit 60b89e1 into master Apr 30, 2025
6 checks passed
@sonnyp sonnyp deleted the no-gps-overclocking-on-planktoscope-hat branch April 30, 2025 13:22
@sonnyp
Copy link
Copy Markdown
Collaborator Author

sonnyp commented Apr 30, 2025

This will be a blocker for #583

We will have 3 options

Remove GPS support

Do people use the GPS hat? It seems like a lot of related things were removed over time.

Remove adafruithat support

And make the release before #583 the latest release to support it

I understand a lot of PlanktoScope with adafruithat are out there and being used so it seems premature. Maybe we can find a way to

  • make maintenance and testing easier
  • call for help

I have the project to have automated test suites that run on real hardware, we could add an adafruithat to the test bench.

Change how hardware configuration works

See #583 (comment) for context

If we extend device-portal with hardware configuration we can do hardware/software setup at runtime rather than build time but this will be significant work

  • scripts will need to be atomic (as much as possible)
  • scripts will need to be idempotent
  • scripts will need to have install/remove steps

On the other hand, it would be good to have this for developer experience anyway.

@ethanjli
Copy link
Copy Markdown
Collaborator

Here are some relevant comments from my notes for past meetings:

  • 2023-10-19 software meeting: according to @chevreuill3000, "the GPS recorded by the machine doesn’t correspond to the location used for sampling - it’s basically useless for GPS location."
  • 2023-08-31 software meeting: according to @fabienlombard, "we don’t need to worry too much about GPS feature - currently it’s 100% useless. It could be useful if the PlanktoScope was able to actually acquire unfiltered water so that we could use it on an inline system (like IFCB). What is more important is to have an accurate system time."

To add some nuance/context to my summaries of these comments:

  • The GPS location of the PlanktoScope sometimes (but does not always) correspond to the location where samples were obtained. If a sample was collected in one place and then imaged later with a PlanktoScope at a different place, the PlanktoScope's GPS coordinates are completely useless. On the other hand, if a sample was collected and then imaged immediately on-site at the same location, the GPS coordinates are useful. I don't know how often the former use-case occurs vs. the latter use-case - maybe @kitazy52 will have some relevant knowledge based on usage patterns in Japan.
  • One of the major projects for future PlanktoScope functionality is to add a capability for using the PlanktoScope on an inline system (so that the PlanktoScope is continuously imaging samples exactly when/where it is collecting the water). I believe @babo989 and @chevreuill3000 are involved in conversations regarding such plans. I don't know how GPS-related information will be collected and recorded in such a scenario - but I assume that if a GPS module is included in future PlanktoScope systems, the GPS module will be not be integrated via Adafruit's GPS HAT.

Removing GPS support would also help make the Adafruit HAT and PlanktoScope HAT versions of the Node-RED dashboard more similar to each other in preparation for a future unification (though if the v3 dashboard is completed first, then maybe we don't need to worry about unifying our current/v2 dashboards).

Based on the above information, I think it's reasonable to deprecate GPS support in v2025.0.0. After I finish #590, I think the main user impact of removing GPS support will be to require manual user input of GPS coordinates, which I think is an acceptable impact. However, it would be nice to get an updated second opinion from users who rely on the Adafruit HAT (e.g. @kitazy52 ) so that we can have a chance to be made aware of any other potential impacts we haven't considered thus far.

If we want to fully remove GPS support in v2025.0.0, I think we should have a discussion to weigh potential impacts of that removal together with the general rule I've been following for PlanktoScope OS release planning that "In general, user-facing parts of the software should only be removed if they were deprecated in a previous YYYY.minor release.". Maybe we'll end up deciding to relax that rule in this specific case or in general...I'm open to possibilities.

If we want to include #583 in v2025.0.0 without a full removal of GPS support in v2025.0.0, what I would propose in v2025.0.0 is to install the necessary programs for GPS support in the OS image, but leave the relevant systemd services disabled, and provide a (deprecated-at-birth) enablement script in the OS image; then the user could make the necessary changes to enable GPS support (even without internet access) by running the enablement script in the Cockpit Terminal, similar to the UX for enabling developer mode. In other words, for v2025.0.0, GPS support would completely be disabled by default (but still accessible by opt-in action), as part of v2025.0.0's complete deprecation for GPS support; this will also help us get some information on whether any users are relying on the GPS HAT in practice, based on the amount (or absence) of users asking for help. Then for a subsequent release of PlanktoScope OS, GPS support could completely be nonexistent on the OS image: we wouldn't install any of the necessary programs when we build the OS image, and we wouldn't provide the enablement script either.

@sonnyp
Copy link
Copy Markdown
Collaborator Author

sonnyp commented Apr 30, 2025

If we want to include #583 in v2025.0

Not from my side and it wouldn't be ready on time anyway.

sonnyp added a commit that referenced this pull request May 13, 2025
ethanjli added a commit that referenced this pull request May 20, 2025
As part of #182, this PR enables the user to switch between the
adafruithat vs. planktoscopehat program variants by editing the SD
card's `~/PlanktoScope/config.json` file and then rebooting (or
restarting `nodered.service` and then restarting
`planktoscope-org.device-backend.controller.service` after Node-RED
becomes ready to receive MQTT messages, without a full system reboot). A
minor caveat here is that GPS functionality is not available on
planktoscopehat-based images running the adafruithat program variants
(because of #591); but in the [2025-05-14 software
meeting](https://docs.google.com/document/d/1AN1jaIdfKChKjMnzSd3ecjRTZGOnoglBfV9LCpevMgk/edit?tab=t.geukyqpm9ntj#heading=h.ziivhaa9au6q)
I plan to propose deprecating GPS functionality from the adafruithat
variants too.

Specifically, this PR:

- unifies the systemd services for the Python device controller into a
single service, and this PR provides a single top-level entrypoint
script which selects the adafruithat vs. planktoscopehat `main.py`
script based on the ~configuration in
`/usr/share/planktoscope/installer-config.yml`, in the same way that
#573 selects adafruithat vs. planktoscopehat for the Node-RED dashboard~
`~/PlanktoScope/config.json` file's `acq_instrument` field (whose values
are like `PlanktoScope v2.6`). See
#583 (comment)
and the [2025-05-07 software
meeting](https://docs.google.com/document/d/1AN1jaIdfKChKjMnzSd3ecjRTZGOnoglBfV9LCpevMgk/edit?tab=t.geukyqpm9ntj#heading=h.81oklfxjwmel)
for additional context on this step towards a piecemeal incremental
unification of the adafruithat vs. planktoscopehat variants of the
Python device controller.
- updates the Node-RED dashboard's `settings.js` file to choose between
the adafruithat vs. planktoscopehat variants of the dashboard based on
the `~/PlanktoScope/config.json` file's `acq_instrument` field.
- fixes a regression in the `fairscope-latest` image caused by #573,
where the `fairscope-latest` image was unable to select the
`planktoscopehat` Node-RED dashboard (because it was looking for a
`fairscope-latest` variant of the dashboard).

This PR also removes OS setup script support for "segmenter-only"
images, since those images were never officially supported (and never
documented in
https://docs.planktoscope.community/reference/software/subsystems/installation/)
and we stopped building them with #573.

TODOs:

- [x] Move the `if __name__ == "__main__"` code blocks in the
adafruithat/planktoscopehat `main.py` files into a main function, so
that we can invoke those scripts more cleanly, and so that we can make
loguru-based logging work properly. This task is blocked by #600 due to
the risk of messy merge conflicts with that PR.
- [x] Move the loguru-based logger from the adafruithat/planktoscopehat
`main.py` files to the hardware controller's top-level `main.py` file,
so that log messages will be saved to file.
- [x] Ensure that uncaught exceptions in the code are are logged to file
by a top-level exception catcher via loguru.

For future PRs (since this PR already touches a lot of files):

- [ ] Move the `.../control/planktoscopehat/planktoscope/identity.py` &
`.../control/planktoscopehat/planktoscope/mqtt.py` &
`.../control/planktoscopehat/planktoscope/camera` &
`.../control/planktoscopehat/planktoscope/imaging` driver modules to a
shared, unified location (perhaps `../control/drivers`) which is used by
both `.../planktoscopehat/main.py` and `.../adafruithat/main.py`,
instead of having two (supposedly) identical of each module.
- [ ] Move everything in `.../control/{variant}/planktoscope` to
`.../control/{variant}`, to reduce redundancies the path names

---------

Co-authored-by: Sonny Piers <sonny@fairscope.com>
sonnyp added a commit that referenced this pull request Jun 18, 2025
Follow up from #591

Dependency of #663
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