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

I2C support for Expert Sleepers Disting EX. #360

Merged
merged 9 commits into from Jan 23, 2021

Conversation

@CarlColglazier
Copy link
Contributor

@CarlColglazier CarlColglazier commented Aug 16, 2020

I don't think this is done quite yet, but I would love to hear input and feedback. I've tested many of these commands on my Crow with success.

i2c address of 0x41-0x44 chosen for compatibility with teletype.

@trentgill
Copy link
Collaborator

@trentgill trentgill commented Aug 28, 2020

This seems like a good addition. Currently you have it all as just raw int types, but perhaps there is a way to leverage crow’s in-built floating types. Eg: You can use ‘s16V’ to map 1.0 in lua to 1638.3 which is the standard mapping (from teletype) for ints to volt-per-octave.

Let me know if you need help with this! There’s examples in jf.lua for example.

@trentgill
Copy link
Collaborator

@trentgill trentgill commented Sep 11, 2020

@CarlColglazier did you see the above message about the s16V type? Have you tested all of the commands?

@CarlColglazier
Copy link
Contributor Author

@CarlColglazier CarlColglazier commented Sep 11, 2020

@trentgill Yes, thank you. I do plan to move some of the types to s16V. Just haven't had the time to mess with it yet. :)

@CarlColglazier
Copy link
Contributor Author

@CarlColglazier CarlColglazier commented Oct 15, 2020

Disting EX v1.4 is out now: https://www.expert-sleepers.co.uk/distingfirmwareupdates.html

I'll be doing some tests over the weekend.

Copy link
Collaborator

@trentgill trentgill left a comment

made the one comment inline, but in general just wanted to check in if this PR is well tested & ready to be merged?

i'm putting together a small point update to officially add w/2.0 i2c commands, and updating the i2c driver timings. would be great to get this included if it's ready.

lua/ii/disting.lua Outdated Show resolved Hide resolved
@trentgill
Copy link
Collaborator

@trentgill trentgill commented Jan 23, 2021

Ok, looks good to me. Why don't we include it in the next version as 'beta' support, owing to the fact that I don't think anyone else has tested it. I wonder if more of the datatypes could be s16V or if we need an additional type or 2.

I really want to keep the value representation as 'crow native' as possible, rather than requiring people to know that values go up to (eg) 16383. If (0,16383) represents 'none' to 'maximum', it seems most crow-like to use values (0.0, 1.0) and have them converted to s16 behind the scenes. This could be a 's16F' for 'fract' type.

I digress though. Can you confirm this is good enough to get into the main branch? Hopefully once it's available in the core branch we'll have more people try it out and report any issues.

@CarlColglazier
Copy link
Contributor Author

@CarlColglazier CarlColglazier commented Jan 23, 2021

I can confirm these changes (1) build (2) don't break any of the scripts I've used them on and (3) return the data from the 'get' functions as expected.

I agree that a s16F type would make for a much more natural experience on crow. As the underlying API will likely change in the future, these changes should probably be described as beta / unstable.

Edit: If you do merge this branch, I'd suggest squashing the commits as I made quite a few commits over time with small changes and it would probably be easier to track as one commit.

@trentgill trentgill merged commit 931e0d9 into monome:main Jan 23, 2021
@trentgill
Copy link
Collaborator

@trentgill trentgill commented Jan 23, 2021

Squashed & merged!

I'll open a new issue for the s16F support & will note that this support is beta in the new release. Thanks so much for adding this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants