Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨Add ADI Addressable LED Support #427

Merged
merged 45 commits into from
Oct 12, 2022
Merged

Conversation

omegaStag
Copy link
Contributor

@omegaStag omegaStag commented Oct 4, 2022

Summary:

Adds support for the VEX ADI LED

Motivation:

Self-explanatory

References (optional):

Closes #424

Test Plan:

Test code is in main.cpp. Plug in LEDs to port and run.

  • Test code works?

Copy link
Member

@WillXuCodes WillXuCodes left a comment

Choose a reason for hiding this comment

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

See above

src/devices/vdml_ext_adi.c Outdated Show resolved Hide resolved
src/devices/vdml_ext_adi.c Outdated Show resolved Hide resolved
src/devices/vdml_ext_adi.c Outdated Show resolved Hide resolved
src/devices/vdml_ext_adi.c Outdated Show resolved Hide resolved
src/devices/vdml_adi.cpp Outdated Show resolved Hide resolved
src/devices/vdml_adi.c Outdated Show resolved Hide resolved
src/devices/vdml_adi.c Outdated Show resolved Hide resolved
@Richard-Stump
Copy link
Contributor

For the C++ api, couldn't we have the led object automatically manage the buffer? When the ADILED object is created, the class can automatically generate an RGB buffer from the number of LEDS the strip has, then the we can have an overload that lets the user access individual elements of the led strip:

pros::ADILed ledStrip(port, num_leds);      // Create the led strip object
ledStrip[0] = 0xFF00FF;                              // Set the first pixel to purple
ledStrip.update();                                       // Send the rgb values to the strip

This would be safer for the end user (which is more C++ like), and easier. Would could probably also provide functions for setting a range of pixels to the same value.

@omegaStag omegaStag requested a review from WillXuCodes October 9, 2022 00:02
Copy link

@sy1vi3 sy1vi3 left a comment

Choose a reason for hiding this comment

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

ext_adi_led_init() appears to work perfectly fine, but ext_adi_led_set() and ext_adi_led_set_pixel() do not.

I'm getting weird issues with program hanging, it's not making sense.

I can get lights to turn on by calling ext_adi_led_init() with 1-indexed port numbers, then using the SDK functions directly to set the lights to on.

sy1vi3 and others added 15 commits October 10, 2022 04:29
subtracted 1 from smart_port because it needs to be zero-indexed 🙃
fix ext_adi_led_set and ext_adi_led_set_pixel
Fixed port decrement/increment issue in the c api's init and set functions, as well as the c++ api's addrled ctor

fixed issue with ext_adi_led_set always returning PROS_ERR because (... || buffer_length) is always true

tested changes on actual hardware to make sure they work
forgot the newline at the end
Co-authored-by: Will Xu <[email protected]>
Co-authored-by: Will Xu <[email protected]>
Fixed C and C++ addrled APIs so that they actually work now
Copy link
Member

@WillXuCodes WillXuCodes left a comment

Choose a reason for hiding this comment

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

As shown in a previous PR, this seems to work on hardware now.

@sy1vi3
Copy link

sy1vi3 commented Oct 12, 2022

AWESOME

@WillXuCodes WillXuCodes changed the title ✨Add LED support ✨Add ADI Addressable LED Support Oct 12, 2022
@WillXuCodes WillXuCodes merged commit d43c838 into develop Oct 12, 2022
@WillXuCodes WillXuCodes deleted the feature/3-wire-led-support branch October 12, 2022 01:14
WillXuCodes added a commit that referenced this pull request Oct 18, 2022
* Implement C functions to set buffer, clear buffer

* C layer

* Implement C++ layer

* Typo fix bool

* Add test code, add get_state and set_state

* Test code typooo

* Update vdml_adi.c

* Smart port offset for inherited funcs (digital write and read))

* Fix return type of get_state

* Fix inheritance to ADIPort instead of DigitalOut

* Add newlines

* Change set_state to turn

* Check buffer_length, buffer_length as member

* Add buffer as class member

* Fix typo

* Types and operator[] declaration

* Typo

* Fix some stuff

* Change _buffer from pointer to vector

* Use .data()

* Add cast

* Fix docs, code, and refine API.

* Fix return types

* Revert main

* Add debug txt

* Add more debug text

* Fix value issue with smart port

* Fix debug prints

* Enhance debug statements

* fix ext_adi_led_set and ext_adi_led_set_pixel

subtracted 1 from smart_port because it needs to be zero-indexed 🙃

* Clean code up

* fix port claiming on set

* Fix accidental decrement

* Fixed C and C++ addrled APIs so that they actually work now

Fixed port decrement/increment issue in the c api's init and set functions, as well as the c++ api's addrled ctor

fixed issue with ext_adi_led_set always returning PROS_ERR because (... || buffer_length) is always true

tested changes on actual hardware to make sure they work

* put gitignore back like it was

oops

* fix gitignore again

forgot the newline at the end

* Update src/devices/vdml_ext_adi.c

Co-authored-by: Will Xu <[email protected]>

* Update version

Co-authored-by: Will Xu <[email protected]>

* Update include/api.h

Co-authored-by: Will Xu <[email protected]>

* remove string include

* fixed indent

* fix spacing in for loop

Co-authored-by: Will Xu <[email protected]>
Co-authored-by: Sylvie <[email protected]>
Co-authored-by: Will Xu <[email protected]>
@WillXuCodes WillXuCodes mentioned this pull request Oct 18, 2022
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.

3.7.2: Add LED support
4 participants