Repository navigation
Conversation
There was a problem hiding this comment.
This is really great work, Shawn, very comprehensive, thank you. Most of the bigger comments I have I think are to bring more of a perspective from the newbie user's side, as opposed to a device-description perspective.
I’ll provide my review in this format and with annotations on the rendered webpage using hypothes.is, like we discussed. I'm not done reviewing yet, I’ll let you know and update this when I am. Update: finished and attached my annotations here for reference.
Here, I’ll write overarching comments that impact several articles and comments that need to be taken into consideration for all docs. We can continue the discussion on the thread if needed. Some comments require more extensive work, which we can address separate from the main review, and I’ve listed them separately below.
Overarching to these docs:
-
It took me a while to get adjusted to the “peripheral ports” terminology, because I primarily think of them as “poke ports”. I understand why you went down this route, since the breakout and rotary encoder use the same ports as the mice poke. However, the plug-and-play aspect is specifically about the mice poke peripheral - the breakout allows customization thus is not plug-and-play, and the rotary encoder was only ever intended as a demo part of a starter kit, not for precise measurements. And I don’t think there is ongoing work on other peripherals targeting the RJ45 ports. These to me are primarily “poke ports”, and a key feature of the system for the user, thus: “3 ports for plug-and-play mice pokes” is stronger than "3 peripheral ports for plug-and-play peripherals.". The breakout is to give them an alternative use, and the rotary encoder is a one-off, not “specialized peripheral” first-class citizen. I’ve discussed this with the developers (Filipe) to make sure we represent what they intended. The decision will impact the terminology and how the mice pokes, rotary encoder and breakout are presented in the docs.
-
Sections on exposed vias in peripherals labelled “for external use only” are muddled (mice poke, rotary encoder). My understanding is their intended use is only as a copy of the signal that is going into the behavior board via the connector, i.e. secondary/auxiliary outputs, or external inputs in the sole case of the DIO1. And that the +5V lines only expose the existing supply line.
Even though it’s possible to do what you’ve described (using the pads to control the peripheral with another acquisition system, or to connect a rotary encoder different than the onboard one to reuse the peripheral port), this should not be part of the docs. I’ve confirmed this with the developers (Filipe) and want us to use our prerogative for the docs to describe only the intended use of the peripheral when connected to the main Harp device. This would be primarily what you’ve called “tapping”, or adding an additional input in the case of the mice poke.
Doing this will improve docs clarity and avoid running into things like “do not power both at the same time” and “Do not turn the onboard encoder while an external one is wired”. Advanced users are welcome to hack, but they will know what they’re doing.
This impacts the Mice Poke and Rotary Encoder Port and Connections sections
Once this concept is clear in the text, we can figure out what terms to use for the “external use only” label and “tap”, to make the meaning clearer, but I’ll raise that separately if required.
- Related to above: what drew my attention here is that describing the complexity of the auxiliary wiring takes away from the fact that the pokes are plug-and-play, so after this section is re-written, I’d also like to ensure the hierarchy of these options within the rendered layout reflects that.
I suggest keeping the first part about the main uses of the peripheral, with the following paragraphs:
-- Connections for plug and play (as currently written)
-- “No further wiring is necessary” statement
-- DIO1 connection in case of the mice poke
-- Valve replacement in case of the mice poke
Add then a sub-title or a defined separate paragraph for a sub-section about “tapping”, which is clearly a separate use-case.
Basically, instead of the current device-oriented “For wiring to the screw terminals:”, make this section read user-oriented: “Main things you’d want to connect to this peripheral”, “Other connections available if you want to use the signal elsewhere”.
To consider here and for all device docs:
- The overview section is to introduce the system and entice the users with the applications, but it currently reads generic, purely technical. The story is important here: What do those things mean for the applications a typical user wants to achieve? Why would they get excited about the system? Be more explicit to help the reader make the connection between capabilities and biological application. Some ideas:
-- Support for over 15 sensors and actuators which can be combined in real-time, e.g. to precisely time delivery of visual and auditory cues, motion and water rewards to timing of events such as nose pokes or running wheel position.
-- Seamlessly synchronize cameras and monitors with your behavioral arena.
-- Plug-and-play mice pokes designed to provide light cues and water delivery capabilities with little additional setup (no soldering required!).
-- Easily drive servo motors, RGB LEDs, and other devices directly, thanks to integrated control circuitry.
It helps to consider the overview section is from the user perspective, while the specs section is from the device perspective.
- Since we only include Harp boards in the TOC under Peripherals, and on this board some electronic components can be wired directly to the board, the TOC ends up being an incomplete list what can be plugged into the main device. I noticed you did explain the connections to LEDs, RGB LEDs, Cameras and Servos, which makes sense.
Should we add an “Others” or “Accessory” menu item for the electronic components that get attached to the board directly? The specs of these would be shorter but would include specifications that you currently have written in the Connections section, e.g. “Connect a WS2812-type addressable RGB LED”, or the fact that up to two RGB LEDs can be driven.
Update: we've discussed this and decided to go for an intermediate solution. We don't alter the TOC b/c it's not strictly needed, but we make sure to put specs only in one place, the Ports section, to help sustainability. In the Connections section, we assume the reader will go look for the specs in the Ports section or we use a more general term to refer to the item. e.g. for the LEDs, we went for "conventional LEDs" and "addressable RGB LEDs" as a general term, and "LED" and "RGB" for a shorthand of that, while the Ports section has the entire type. We can repeat a spec in the Connections section if strictly necessary for connecting, such as the current rating if a resistor is required.
- Version differences. We'd agreed that the docs would target the latest hardware revision of the hardware being produced. An exception is for the behavior breakout, of which both versions are being produced, so latest didn’t supersede the other.
The device docs are not intended to be a comprehensive account of how to use all versions, only the latest. I suggest we only keep a mention of version differences in cases in which not knowing can cause damage to the device or data corruption or fail silently, but I expect this to be rare.
E.g. the mini usb vs usb-c is not relevant, they will connect whatever cable fits.
The Input (Screw Terminal) block - If they don’t have it they’ll be puzzled at best.
The point of having the hw differences noted in the tables on the device specs pages was to provide this explanation there, but not to track versions changes in every feature.
I recognize work has gone into doing this, but let’s revert for the sake of clarity of the docs and sustainability.
- Related to above: removing the version labels will help clarity in the Connections images. It's also better to use that space to label the ports/connectors with their name, so the text is easier to follow.
-
For Connections, aim for one valid/recommended way of connecting in the image, and add any alternatives in the text. e.g. remove the AD1 in the pic for the photodiode, because you already have it going into AD0. Provide the option in teh text.
Keep the bullet points under the image one per connection (signal, power, gnd) as much as possible.
If presenting two alternatives, consider either repeating the image e.g. photodiode ANA vs DIG wiring, or adding a new tab e.g. LED vs RGB LED. This makes each diagram standalone for one function/device, and so easier to follow. -
Please don’t use our SKU or procurement information in the docs, these docs are agnostic to the manufacturer. If necessary, state that there are different versions available, with or without x feature.
More extensive work required (independent of main content):
-
The main pic is not consistent with the image style we had defined. This will take additional work to be done separately, by taking all three overview images (behavior, sound card, timestamp generator) and unifying them to settle on a style. Let's discuss.
-
The “About These Examples” has some problems with the logic. What is currently written reads as if there are only 3 sections in every example workflow, and names both the “sections” and the 3 types “operating motifs” building blocks. We’ve discussed this and I will rewrite it.
-
How realistic is it to remove terms like "broadcast", "register" and "payload" from the examples (except in property names)? Are they absolutely required for usage? I really think there is an abstraction here that is possible for the sake of onboarding non-technical users. I expect this will need discussion :)
E.g. Instead of:
Consider:
File with annotations:
harp-device.behavior-hypothesis-ceci-comments-2026-10-19_13-37.csv
|
|
|
@banchan86 Thanks for all the revisions. Just a couple of things on my side before merging: The table on the On the version footer, once the first release with a single version number is out, the firmware and package versions will be the same number and known at build time, so we could look into generating that part of the footer as well. No action needed on the notes about the current firmware v3.3 or on the version footer. The next firmware release fixes #41 and #44, so we can update those notes in the same pull request. After this merges I am planning a follow-up that bumps the @ChucklesOnGitHub let us know once you are done with the review so we can merge. |
|
@banchan86 I did a full pass over the docs with the checklist we use before releases, and prepared fixes for the mechanical items so they don't add to your workload. Happy to push them to this branch as separate commits if that works for you, or you can apply them yourself. Fixes ready to push
A full DocFX build with all of these has no warnings. For future diagrams, embedding photos and renders as JPEG in Inkscape, or downscaling them before embedding, keeps the files small. Embedding is still the right choice, since DocFX shows SVGs through Prose A sweep against our docs style guidelines found the prose mostly consistent, with a few recurring patterns we normally change: second person, parenthetical asides, colons and semicolons in the middle of sentences, and around 16 sentences over 30 words. I'd suggest a separate pass after this merges rather than holding the PR on it. Naming The pages under "Bonsai Workflows" are already language-neutral device tasks, so we were wondering whether to call the section "Examples", which would let each page grow Python tabs next to the Bonsai ones as the Python interface matures. Since the soundcard docs follow the same structure, it might be worth agreeing on this for the shared template first. Similarly, once the Python API reference is added, we were thinking of splitting the current Reference entry into a separate Bonsai Reference and Python Reference, which we felt would be easier to navigate than a single combined entry. |
|
@banchan86 Ended up pushing directly here to avoid stacking PRs. We can always revert or merge this before, etc. We can discuss them tomorrow. The most important one here I feel is 4e9ba34 which we should aim to either squash into one of the earlier commits or squash-merge the entire PR so that the history does not accumulate large binary diffs. |
|
@glopesdev, most of those changes were good with us; thanks for raising them!
|
chucklescience
left a comment
chucklescience
left a comment
There was a problem hiding this comment.
The current version of the docs has passed the OE final review stage!
Sentence case is fine for us.
Any remaining points we can discuss in the meeting. I'd suggest we target them on a second iteration at a later stage.
- Rename peripheral ports to poke ports - Document latest hardware version and drop qualifiers - Restructure connections tab instructions to one connection per bullet - Split the LED tab into LED and RGB LED tabs with separate images - Remove peripheral qualifiers in intro - Add note on the Rotary Encoder's intended use. - Add bold connector titles - Remove faded alternative-connection lines - Split photodiode connection images to analog input and digital output - Convert the servo zoom to the pigtail wire style - Flip the LED photo to fix orientation
- Add photodiode switch setting schematic - Reword peripheral auxillary wiring sections - Tighten peripheral feature list - Expand camera setup checklist - Replace landing page feature bullets - Fix LED register range swap - Equalize layout for LED images
- Standardise terminology for digital outputs - Split digital output connection figures and section into logic-input and load sections - Rename sections for advanced configuration - Drop disable branches on mimic article - Edit prose throughout for grammar and clarity
Promote article headings one level so each page starts with an H1, which DocFX uses as the page title, and set the landing page title in its metadata.
The version footer and the pulse output warning are no longer published as standalone pages, and still render wherever they are included.
Embedded images are re-encoded, as JPEG where they have no transparency, and the pinout image is downscaled to 1200 pixels wide. The diagrams look the same at the size the pages show them.
Workflows now start with a byte order mark and have no final newline, and the editorconfig gains the YAML and Bonsai sections of the shared template so editors keep that format.
Pending discussion, workflow annotation nodes are also title case
- Substitute Windows.Input for Bonsai.StarterPack in installation step - Change first steps and acquire analog data example - Add screenshot of TimeSeriesVisualizer
- Also updates Bonsai workflow annotation nodes
|
@banchan86 One last thing before the squash merge, since whatever goes into the squashed commit stays in the history for good. The SVGs are all small now, so the largest images left are the product photos. The
Downscaling them to 900 px wide, which is twice the displayed width so they stay sharp on high-density screens, and saving at JPEG quality 85 brings the first four down to 30 to 60 KB each, which should look the same at the size the pages show them. If the full-resolution originals are useful elsewhere, maybe they could live with the hardware assets discussed in #47 rather than in the docs. Happy to push this as one more commit if that works for you, otherwise feel free to apply it on your side before we merge. |
Photos are resized to twice their displayed width and saved as JPEG, and file names describe what each photo shows. The author and the CC BY-SA 4.0 license are recorded in the EXIF metadata of each photo.
Summary
Builds out the complete documentation site for the Behavior board on the existing docfx scaffold: a landing page, device and peripheral specs, a "Getting Started" section, Bonsai "Examples", data management and troubleshooting, and register table and overlays in the "Reference".
Articles
index.md) rewritten, with a new hero figure of the board wired to the PC and its peripherals.behavior.md(key features, specs, hardware and firmware version tables) and one page per peripheral underarticles/peripherals/(Mice Poke, Breakout, Rotary Encoder, Speaker, Photodiode).ports-connections.mdwith a port reference, an annotated pinout and one tabbed wiring diagram per connection scenario;installation.md(Bonsai, Python, GUI, firmware update);behavior-gui.md;bonsai.mdwith a first-steps workflow.about-these-examples.mdexplaining the shared workflow pattern, then eleven articles, one per functional group: acquire analog data, read digital inputs, control digital outputs, generate PWM, control LEDs, control poke, mimic poke signals, track rotary encoder, trigger cameras, control servo motors, advanced configuration. Each embeds copyable.bonsaiworkflows (71 files underdocs/workflows) and uses keyboard-triggered commands plus visualizer sections for event registers.logging-analysis.md) and Troubleshooting (errors.md: indicator lights, COM port and device errors).docs/includes/(version footer, output-pulse warning).Reference
docs/apidoc/attach Bonsai examples to the generated register and payload pages, with per-port variants so sibling registers get an overlay too.harp-schema-processorFigures
Environment
.bonsai/Bonsai.configaddsBonsai.Windows.InputandBonsai.Design.Visualizers, needed by the example workflows.editorconfigupdated for Bonsai files