Skip to content

Improve MQTT API reference docs#402

Merged
ethanjli merged 27 commits intofairscope:masterfrom
melissadjadoun:mine
Jul 24, 2024
Merged

Improve MQTT API reference docs#402
ethanjli merged 27 commits intofairscope:masterfrom
melissadjadoun:mine

Conversation

@melissadjadoun
Copy link
Contributor

@melissadjadoun melissadjadoun commented Apr 25, 2024

this PR updates the MQTT documentation, adding general architecture, json message content explanations for each topic and interval tables to limit value intervals

@melissadjadoun melissadjadoun changed the title Mine Update and Expand MQTT Docs May 7, 2024
@ethanjli
Copy link
Collaborator

@melissadjadoun As promised in today's meeting, I've fixed the GitHub Actions workflow checks now!

@melissadjadoun
Copy link
Contributor Author

@ethanjli I've made the changes I did to the documentation: I've tried to include all possible error and status messages for each action, along with a description, and also the contents of the imager config metadata file.

@ethanjli ethanjli self-requested a review May 30, 2024 14:41
Copy link
Collaborator

@ethanjli ethanjli left a comment

Choose a reason for hiding this comment

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

What we have so far is a good start; in this review I've identified various major things which need changes, but I haven't commented on smaller changes needed to make this page clearer and more authoritative - we'll address that in another review after you address the changes I've requested in this review.

Before my next review of this pull request, please compare your documentation page to the draft documentation I had already written in https://docs.google.com/document/d/1BxAlUBMRXEhJkk6OMDVYN7z7l7mky7YTwA_C2Uykrm4/edit#heading=h.fmuu43juukhx , to identify any information I had written there which is still an accurate description the current state of the Python backend (some information may be outdated) but which is missing from your documentation page; and then edit your documentation page to ensure it is not missing that information.

Please also read https://diataxis.fr/reference/ and use that as a writing/style guide as you edit/polish your writing on this page.

@melissadjadoun
Copy link
Contributor Author

@ethanjli I've made all the changes you've asked for in the documentation. I've also conducted the tests I could on the remote planktoscope to check the logs. The light part isn't a final version yet, so please let me know if there are any further changes needed.

@ethanjli ethanjli changed the title Update and Expand MQTT Docs Improve MQTT API reference docs Jun 26, 2024
@ethanjli
Copy link
Collaborator

ethanjli commented Jun 27, 2024

@melissadjadoun I've reviewed the updated version of the MQTT API documentation, and I:

  • Simplified the overview
  • Changed the heading structure of the page (i.e. the division of subsections) to be simpler and more consistent, for a better table of contents
  • Fully edited the "Pump" section to meet my standards

Can you please edit the other sections of the document to match the structure, formatting, and wording of the "Pump" section as much as possible? For now, please don't edit the headings themselves (i.e. everything in the page's table of contents) - only edit the text under each heading.

Also, can you please remove status/error values which are not actually in the code? For example, the "Error, invalid_direction" status/error (which you had listed for the Pump and Focus sections) does not actually exist in https://github.com/PlanktoScope/device-backend. We can instead add notes on the page about missing error values (e.g. in the Pump section, I wrote: "Note: the MQTT API does not yet completely specify error messages in response to invalid values for the direction, volume, and flowrate parameters.")

@melissadjadoun
Copy link
Contributor Author

@ethanjli I have edited the other sections of the documentation to match the structure, formatting, and wording of the "Pump" section as closely as possible. I also removed any status/error values that are not actually present in the code.
The changes have been pushed. Please let me know if there's anything else you'd like me to adjust.

@ethanjli
Copy link
Collaborator

In my latest commit, I've fixed a bunch of structure/formatting/language inconsistencies and factual errors which you had missed. I think this PR is now good enough to merge; we can open new PRs in the future to fix any remaining errors which I've missed so far.

@ethanjli ethanjli merged commit 89302ff into fairscope:master Jul 24, 2024
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