Arduino: Adds analog sequences - #967
Open
nicost wants to merge 7 commits into
Open
Conversation
min and max voltage per channel.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds firmware v6 analog-output ranges and triggered DA sequences to the Arduino adapter.
Changes:
- Queries firmware-defined DA voltage ranges and resolution.
- Adds upload/start/stop support for analog sequences.
- Adds MCP4728 firmware support.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
Arduino.h |
Declares DA range and sequencing state/APIs. |
Arduino.cpp |
Implements range discovery and sequence control. |
AOTFcontroller.ino |
Implements firmware v6 protocol and DAC sequencing. |
Suppressed comments (4)
DeviceAdapters/Arduino/Arduino.cpp:478
- This accepts a partial response containing only byte 37 and publishes a maximum sequence length of zero. Check that all three bytes were received, as the existing capability queries above do.
if (seqLenAnswer[0] != 37)
return ERR_COMMUNICATION;
DeviceAdapters/Arduino/Arduino.cpp:1477
- The new wire format explicitly supports signed voltage ranges, but rejecting
maxV_ <= 0makes a valid negative-only range (for example, -10 V to -1 V) unusable. Validate that the reported span is positive instead of requiring a positive upper bound.
double span = refMax - refMin;
long value = (span > 0.0) ? (long) ((volts - refMin) / span * numSteps_) : 0;
if (value < 0) value = 0;
if (value > (long) numSteps_) value = (long) numSteps_;
if (maxV_ <= 0.0)
return DEVICE_INVALID_PROPERTY_VALUE;
DeviceAdapters/Arduino/Arduino.cpp:1562
- The upload reply is considered complete based only on its echo bytes. In particular, an empty upload succeeds if only those first two bytes arrive, while a truncated non-empty reply is misreported as a firmware rejection. Require all four bytes before examining
storedCount.
if (answer[0] != 38 || answer[1] != (unsigned char) (channel_ - 1))
return ERR_COMMUNICATION;
DeviceAdapters/Arduino/Arduino.cpp:1619
- A response containing only the command byte is accepted because the missing transition-count byte remains zero. Check
bytesReadbefore reporting a successful stop and logging a fabricated count.
if (answer[0] != 43)
return ERR_COMMUNICATION;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+443
to
+444
| if (answer[0] != 36 || answer[1] != ch0) | ||
| return ERR_COMMUNICATION; |
Comment on lines
+1504
to
+1508
| int CArduinoDA::AddToDASequence(double voltage) | ||
| { | ||
| if ((long) sequence_.size() >= daMaxSeqLength_) | ||
| return DEVICE_SEQUENCE_TOO_LARGE; | ||
| sequence_.push_back(voltage); |
Comment on lines
+1685
to
+1688
| std::istringstream is(seq[i]); | ||
| double v; | ||
| is >> v; | ||
| int ret = AddToDASequence(v); |
Comment on lines
+1756
to
+1760
| else if (eAct == MM::AfterSet) | ||
| { | ||
| std::string state; | ||
| pProp->Get(state); | ||
| sequenceOn_ = (state == g_On); |
Comment on lines
+301
to
+302
| Wire.begin(); | ||
| mcp_ok = mcp.begin(MCP4728_ADDR); |
Comment on lines
+1379
to
+1384
| if (hub->GetDAVoltageRange(channel_, physMinV_, physMaxV_, numSteps_)) | ||
| { | ||
| hasPhysRange_ = true; | ||
| minV_ = physMinV_; | ||
| if (maxV_ > physMaxV_) | ||
| maxV_ = physMaxV_; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bumps firmware version to 6.
Let's firmware set analog output voltage range (min, max, and number of steps).