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

refactor i2c #36

Merged
merged 3 commits into from Jun 3, 2018
Merged

refactor i2c #36

merged 3 commits into from Jun 3, 2018

Conversation

@scanner-darkly
Copy link
Member

@scanner-darkly scanner-darkly commented May 12, 2018

  • fixed addresses (addresses are 7 bit, not 8 bit, cleared MSB)
  • split commands
  • added comments / reformatted

see https://llllllll.co/t/teletype-i2c-protocol/13642 for discussion

@scanner-darkly
Copy link
Member Author

@scanner-darkly scanner-darkly commented May 12, 2018

@tehn @catfact @trentgill please take a look

@scanner-darkly
Copy link
Member Author

@scanner-darkly scanner-darkly commented May 12, 2018

Travis build failed due to older firmwares needing to be updated to be used with the latest libavr32

@tehn
tehn approved these changes May 14, 2018
Copy link
Member

@tehn tehn left a comment

thanks for pointing this out.

i'm good for a more radical re-assignment of the monome stuff. ie

WW 0x10
MP 0x11
ES 0x12

and there's not a great reason that there are even-numbered addresses for ansible modes. not sure what my rationale was earlier.

@scanner-darkly
Copy link
Member Author

@scanner-darkly scanner-darkly commented May 14, 2018

that would mean newer versions of teletype wouldn't be backwards compatible and would require updating all the other firmwares as well - perhaps more trouble than it's worth?

@tehn
Copy link
Member

@tehn tehn commented May 14, 2018

@trentgill
Copy link

@trentgill trentgill commented May 14, 2018

I believe 'only even addresses' was because we thought it was left-aligned 7bit number?
Regardless this seems like a positive change as it won't break anything, and means the code reflects what is happening on a hardware level.
I will update the JF & W/ repos which will make those defines a lot more meaningful!

@bpcmusic
Copy link

@bpcmusic bpcmusic commented May 14, 2018

Thanks for taking the lead on this, @scanner-darkly. You really are the best!

It looks like we need to add in the range for the TELEX modules (0x60-0x6F), fix the conflict with the 16n (FADER), fix the conflict with the ER-301, and add in the tentative addresses for the Tetrapad integration that I've proposed in specification with kisielk. I'll suggest some things over on the forum shortly.

@scanner-darkly
Copy link
Member Author

@scanner-darkly scanner-darkly commented May 14, 2018

@bpcmusic - sorry, should've tagged you in this PR! i'll do another commit to include your changes.

@scanner-darkly
Copy link
Member Author

@scanner-darkly scanner-darkly commented May 16, 2018

@bpcmusic added your changes, i expanded the names to avoid potential conflicts, let me know if they look okay. er-301 change will be in a separate PR.

@scanner-darkly
Copy link
Member Author

@scanner-darkly scanner-darkly commented Jun 2, 2018

@bpcmusic @tehn are we good to merge this?

@tehn
Copy link
Member

@tehn tehn commented Jun 2, 2018

looks great!

@scanner-darkly scanner-darkly merged commit 46c2b0d into monome:master Jun 3, 2018
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@bpcmusic
Copy link

@bpcmusic bpcmusic commented Jun 4, 2018

I'll test tomorrow with the latest; apologies for not responding sooner. I've been in SMD hell this weekend and am still half-delirious from the fumes. Thx@

@scanner-darkly
Copy link
Member Author

@scanner-darkly scanner-darkly commented Jun 4, 2018

@bpcmusic no worries, i need to do another pull request for the er-301 changes and figured i could just add any other required changes there. would you need an updated teletype firmware for testing? i’ll update my tt branch to use this version tomorrow.

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

4 participants