Skip to content

Expand digital/PWM support, add validation and robust SCPI parsing, update README - #2

Open
GiuseppeLeviBo wants to merge 1 commit into
mainfrom
codex/controlla-codice-per-gestione-led-pin13-mcdirg
Open

Expand digital/PWM support, add validation and robust SCPI parsing, update README#2
GiuseppeLeviBo wants to merge 1 commit into
mainfrom
codex/controlla-codice-per-gestione-led-pin13-mcdirg

Conversation

@GiuseppeLeviBo

Copy link
Copy Markdown
Owner

Motivation

  • Add support for additional digital pins (including LED_BUILTIN) and make PWM/servo handling more robust and safe.
  • Prevent invalid channel usage by centralizing validation and parsing logic for analog/digital/PWM channels.
  • Improve SCPI command error handling and make commands accept both SCPI indices and real Arduino pin numbers.

Description

  • Extend digitalPins to include D2..D10 and LED_BUILTIN and compute digitalPinCount and pwmPinCount dynamically.
  • Introduce utility helpers (isValidAnalogChannel, isValidDigitalChannel, isValidPwmChannel, findPwmChannelByPin, parseChannel, resolveDigitalChannel) to validate and parse channel arguments.
  • Harden processCommand by validating inputs, returning ERR on invalid parameters, and supporting pin-number addressing for DIG:OUT queries and commands.
  • Improve PWM and servo interplay by tracking pwmValue and servoAttached, detaching servos before setting PWM, and attaching servos only when SOUR:SERVO is used.
  • Revise resetDevice to correctly reset/detach servos, clear PWM and digital states, and use the new counts; bumped identification string version to 1.2.
  • Update READ?, ROUT:SCAN?, MEAS:VOLT?, MEAS:RAW? and related parsing to use the new validation and parsing helpers.
  • Update README.md to document the expanded digital pin mapping, new examples showing DIG:OUT with pin numbers, and clarify PWM pins behavior.

Testing

  • Built the Arduino sketch with the Arduino CLI for avr:uno to verify it compiles successfully.
  • No automated unit tests are present in the repository, so behavior was enforced via compile-time checks only.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da3bd3c8c0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Arduino_SCPI.c
Comment on lines +69 to +72
if(isValidDigitalChannel(parsed))
{
channel = parsed;
return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Prefer physical pin matches before treating <ch> as index

resolveDigitalChannel accepts any token in 0..9 as a SCPI index before it ever checks digitalPins, so the new pin-number addressing described in README.md:155-157 is wrong for overlapping values. On Uno, DIG:OUT 2,1 drives digitalPins[2] (D4) instead of physical D2, and DIG:OUT 9,1 hits digitalPins[9] (D13) instead of D9. As written, physical D2-D9 can never be addressed even though this commit advertises that they can.

Useful? React with 👍 / 👎.

Comment thread Arduino_SCPI.c
Comment on lines +408 to +410
pwmValue[ch] = val;
analogWrite(pwmPins[ch], val);
Serial.println("OK");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Synchronize DIG:OUT? state when PWM drives D9/D10

Now that D9/D10 are exposed through the digital channel map, SOUR:PWM also needs to keep digitalState consistent. This block only updates pwmValue and the hardware pin, so after SOUR:PWM 0,255 a subsequent DIG:OUT? 7 still returns the stale boolean from the last DIG:OUT call (typically 0 after reset) even though D9 is being held HIGH. The shared digital/PWM behavior documented in README.md:157-163 therefore does not round-trip for the PWM pins.

Useful? React with 👍 / 👎.

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.

1 participant