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

Fix for SPI overflow error. #6

Closed
wants to merge 1 commit into from
Closed

Fix for SPI overflow error. #6

wants to merge 1 commit into from

Conversation

julienrhodes
Copy link
Contributor

SPI write commands fail when the line length in silta exceed 63 characters
(presumably the receive buffer size on the hardware). These commands are sent in
the form "spi FF FF FF ... FF\n" where FF FF corresponds to ascii hex
representations of the byte list (i.e. [255, 255, 15] -> "spi FF FF F\n".
Worst-case, each byte requires three chars and there's always a prefix of "spi "
and a newline suffix.

SPI write commands fail when the line length in silta exceed 63 characters
(presumably the receive buffer size on the hardware). These commands are sent in
the form "spi FF FF FF ... FF\n" where FF FF corresponds to ascii hex
representations of the byte list (i.e.  [255, 255, 15] -> "spi FF FF F\n".
Worst-case, each byte requires three chars and there's always a prefix of "spi "
and a newline suffix.
@alvarop
Copy link
Owner

alvarop commented Jan 11, 2017

Thanks for the PR!
The issue is over here it seems: https://github.com/alvarop/silta/blob/master/fw/console.c#L27
Not sure how I left that so small!

The USB-serial rx buffer is 512 bytes (https://github.com/alvarop/silta/blob/master/fw/usbd_cdc_vcp.c#L38)

I saw we bump up both to 4096 bytes! There's plenty of RAM to spare.

I don't have time to test it right now, but if you want to change those two values and update the error checking code, I'll merge it in and do a release.

@julienrhodes
Copy link
Contributor Author

Sure! Thanks for pointing out where the cmdBuffer is defined. I'll play with that, but I think the best solution also involves splitting up longer SPI command strings and sanity checking the __send_cmd method's string lengths prior to attempting a write. The cmdBuffer could be left at a length of 64 - as long as the python sw knows about the limitation. It probably sounds unnecessary at first glance, but it would be nice to support arbitrarily long SPI commands in case somebody wanted to drive thousands of LEDs on an LED strand. What is the best way to import the cmdBuffer's size into python? I've seen people use h2py to make .h variables accessible in python. Any better ideas?

@alvarop
Copy link
Owner

alvarop commented Jan 12, 2017

Arbitrary length SPI commands are possible, but the firmware will have to be re-written to support them.
Right now, it will block while doing a single command. It would have to be doing SPI using DMA and be able to request more data while it is currently sending.

A good temporary compromise would be a bigger buffer that takes care of most use-cases until there's time to re-architect :D

As for getting data back into python... There's currently a command to get the serial number as well as the firmware version. I'm imagining a more generic 'get_property' command, which can get key-value pairs, and would allow a Silta device to advertise it's capabilities without having to share .h files.

@alvarop
Copy link
Owner

alvarop commented Jan 13, 2017

Just did a release with a fix. Should allow 1024 byte transactions. Also checking bounds in python.

https://github.com/alvarop/silta/releases/tag/releases%2Fv0.1.5

@julienrhodes
Copy link
Contributor Author

Nice!

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.

None yet

2 participants