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

Fixes #179 - I2C tutorial added #181

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mittalshravika
Copy link
Contributor

Fixes #179. I2C tutorial with relevant code and fritzing diagram added.

Copy link
Contributor

@HipsterBrown HipsterBrown left a comment

Choose a reason for hiding this comment

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

This is a great start! I would really like to learn more about why certain values are used and how the 12 bit conversion works in the sample code. Think about reading that code with no knowledge of working with hardware, and how would a reader apply this to another I2C project.

For the sample code, how could we continually read values while the user interacts with the Tessel?

Tutorials/i2c.md Outdated
});
```

[Datasheet for accelerometer module](http://www.nxp.com/docs/en/data-sheet/MMA8452Q.pdf)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you include a link to a resource where folks can learn more about I2C communication, i.e. SparkFun?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for the reviews!! We will update the PR with the requested changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The link to sparkfun for I2C Communication has been added.

Tutorials/i2c.md Outdated

// Connect to device
var port = tessel.port.A; // Select Port A of Tessel
var slaveAddress = 0x1D; // Specefic for accelerometer module
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did you find this address? From somewhere in the datasheet? How can readers discover this as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been added.

Tutorials/i2c.md Outdated
i2c.read(numBytesToRead, function (error, dataReceived) {

// Print data received (buffer of hex values)
console.log('Buffer returned by I2C slave device ('+slaveAddress.toString(16)+'):', dataReceived);
Copy link
Contributor

Choose a reason for hiding this comment

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

What should I expect back from this read? Would anything come back from this component?

Tutorials/i2c.md Outdated
});

// Read/Receive data over I2C using i2c.transfer
i2c.transfer(new Buffer([0x0D]), numBytesToRead, function (error, dataReceived) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you choose to use 0x0D for this Buffer value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been added.

Tutorials/i2c.md Outdated
i2c.transfer(new Buffer([0x0D]), numBytesToRead, function (error, dataReceived) {

// Print data received (buffer of hex values)
console.log('Buffer returned by I2C slave device ('+slaveAddress.toString(16)+'):', dataReceived);
Copy link
Contributor

Choose a reason for hiding this comment

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

What could I expect to be logged here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been added.

Tutorials/i2c.md Outdated
//Create a blank array for the output
var out=[];
for (var i=0;i<3;i++)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

This bracket should be on the same line as the for statement, which doesn't need to be so condensed:

for (var i = 0; i < 3; i++) {
  // rest of the iterating code
}

Also why are only 3 iterations needed to read this output Buffer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been added.

Tutorials/i2c.md Outdated
//Now, try to print the accelerometer data using i2c.transfer
i2c.transfer(new Buffer([0x01]), 6, function (error, dataReceived) {

// Print data received (buffer of hex values)
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is a little misleading, because the error is not a "buffer of hex values" to be printed. If there is an error, when you throw it, the program stops with the error message and stack trace as the output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment has been removed.

Tutorials/i2c.md Outdated
for (var i=0;i<3;i++)
{
//iterating for the x, y, z values
var gCount=(dataReceived[i*2] << 8) | dataReceived[(i*2)+1]; //Converting the 8 bit data into a 12 bit
Copy link
Contributor

Choose a reason for hiding this comment

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

What does gCount mean or stand for? Can you add a paragraph after the sample code to explain how converting 8 bit data into 12 bit data works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Frijol Could you please help us out as to what is exactly happening in this part of the code? We tried understanding it from our part but couldnt do so.

Tutorials/i2c.md Outdated
//iterating for the x, y, z values
var gCount=(dataReceived[i*2] << 8) | dataReceived[(i*2)+1]; //Converting the 8 bit data into a 12 bit
gCount=gCount >> 4;
if (dataReceived[i*2] > 0x7F)
Copy link
Contributor

Choose a reason for hiding this comment

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

What does the 0x7F value mean? Why do we care if our dataRecieved Buffer has a value greater than 0x7F?

Copy link
Contributor Author

@mittalshravika mittalshravika Sep 11, 2017

Choose a reason for hiding this comment

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

0x7F in decimal is 127 and it could not be found anywhere in the data sheet as well, What is the significance of this here?

Tutorials/i2c.md Outdated
{
gCount = -(1 + 0xFFF - gCount); // Transform into negative 2's complement
}
out[i] = gCount / ((1<<12)/(2*2));
Copy link
Contributor

Choose a reason for hiding this comment

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

What is happening here?

@mittalshravika
Copy link
Contributor Author

@HipsterBrown, @Frijol, the changes requested have been added expect a few which we have doubt in. Could you please look into the same and help us out with the rest of the changes left.

Tutorials/i2c.md Outdated

// Now, try to print the accelerometer data using i2c.transfer
// The register address for OUT_X_MSB is 0x01. This can be found at Page 19 of https://www.nxp.com/docs/en/data-sheet/MMA8452Q.pdf
// 6 Bytes are used for pairwise MSB and LSB of the x,y and z axis
Copy link
Contributor

Choose a reason for hiding this comment

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

What do MSB and LSB stand for? What do mean by "used for pairwise MSB and LSB"?

@HipsterBrown
Copy link
Contributor

@Frijol Do you think the I2C section of the "Communication Protocols" tutorial should be updated to link to this new tutorial? how much sample code do we keep in that older section?

@mittalshravika
Once you get some clarity from @Frijol, could you add a paragraph (before or after the sample code) about the converting the data returned for each axis from 8-bit to 12-bit and creating the final values being logged. Adding a screenshot or sample of the final output, like you included in the issue, at the end will be helpful for readers to know what to expect at the end.

@mittalshravika
Copy link
Contributor Author

Sure, I will add a section about the process. Thanks a lot for the reviews!!

@Frijol
Copy link
Member

Frijol commented Sep 17, 2017

Yes, a link there would be very helpful! @HipsterBrown I'm not sure what you're referring to with "older section"– do you mean the code sample in the communication protocols file itself? I believe it has been updated to latest standards. Are you suggesting we replace the example code that's there with a link to the tutorial?

@HipsterBrown
Copy link
Contributor

Are you suggesting we replace the example code that's there with a link to the tutorial?

@Frijol Potentially, yes. I guess the sample code in the Communication Protocols section is helpful for explaining the concept.

@brihijoshi
Copy link
Contributor

brihijoshi commented Sep 26, 2017

@HipsterBrown @Frijol The edited commented code has been added :)

We will add the sample output for the code and run the build for the gitbooks.

Please review 😄

@mittalshravika
Copy link
Contributor Author

@HipsterBrown @Frijol The sample output has also been added. Please review the same.

Copy link
Member

@Frijol Frijol left a comment

Choose a reason for hiding this comment

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

This is a strong start! My comments are largely based on the assumption that the reader of this tutorial will want very thorough explanation– ideally, someone new to programming and electronics could read it and understand.

Tutorials/i2c.md Outdated

I2C is a simple-to-use communication protocol which can have more than one master, with the upper bus speed defined, and only two wires with pull-up resistors are needed to connect almost _unlimited_ number of I2C devices.

Each slave device has a unique address. Transfer from and to master device is serial and it is split into 8-bit packets. All these simple requirements make it very simple to implement I2C interface even with cheap micro-controllers that have no special I2C hardware controller. You only need 2 free I/O pins and few simple i2C routines to send and receive commands.
Copy link
Member

Choose a reason for hiding this comment

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

typos: micro-controllers > microcontrollers; i2C > I2C

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Tutorials/i2c.md Outdated

Each slave device has a unique address. Transfer from and to master device is serial and it is split into 8-bit packets. All these simple requirements make it very simple to implement I2C interface even with cheap micro-controllers that have no special I2C hardware controller. You only need 2 free I/O pins and few simple i2C routines to send and receive commands.

Let us get the accelerometer values using the I2C protocol. We would be using the [accel-mma84](https://www.seeedstudio.com/Tessel-Accelerometer-Module-p-2223.html)
Copy link
Member

Choose a reason for hiding this comment

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

We would > We will

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Tutorials/i2c.md Outdated

Each slave device has a unique address. Transfer from and to master device is serial and it is split into 8-bit packets. All these simple requirements make it very simple to implement I2C interface even with cheap micro-controllers that have no special I2C hardware controller. You only need 2 free I/O pins and few simple i2C routines to send and receive commands.

Let us get the accelerometer values using the I2C protocol. We would be using the [accel-mma84](https://www.seeedstudio.com/Tessel-Accelerometer-Module-p-2223.html)
Copy link
Member

Choose a reason for hiding this comment

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

The accel-mma84 what? The MMA84 accelerometer integrated sensor?

Copy link
Member

Choose a reason for hiding this comment

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

Or would it be more useful to say, the Tessel Accelerometer Module?

Copy link
Member

Choose a reason for hiding this comment

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

Would be a good idea to link to the MMA84 datasheet regardless as it will be useful for someone following along

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Tutorials/i2c.md Outdated

Let us get the accelerometer values using the I2C protocol. We would be using the [accel-mma84](https://www.seeedstudio.com/Tessel-Accelerometer-Module-p-2223.html)

![Fritzing Diagram](http://i.imgur.com/zK4U4S3.png)https://github.com/276linesofCode/Codes-Ideas/edit/master/tut-i2c.md#fork-destination-box
Copy link
Member

Choose a reason for hiding this comment

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

Is the Github link supposed to be there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Tutorials/i2c.md Outdated
// Connect to device
var port = tessel.port.A; // Use the SCL/SDA pins of Port A

//This address of the Slave has been taken from https://github.com/tessel/accel-mma84/blob/master/index.js#L15
Copy link
Member

Choose a reason for hiding this comment

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

Typo: space after //

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Tutorials/i2c.md Outdated
positive. If it is negative (checked using 0x7F, which is the maximum possible number that can be made from 7 bits), the if condition changes
it to a 2's complement form, thus making it positive and adding a "-" sign in front of it.

Lastly, normalise the coordinate to get a value between 0 and 1, dividing the gCount by 2^10.
Copy link
Member

Choose a reason for hiding this comment

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

normalise > normalize

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Tutorials/i2c.md Outdated
positive. If it is negative (checked using 0x7F, which is the maximum possible number that can be made from 7 bits), the if condition changes
it to a 2's complement form, thus making it positive and adding a "-" sign in front of it.

Lastly, normalise the coordinate to get a value between 0 and 1, dividing the gCount by 2^10.
Copy link
Member

Choose a reason for hiding this comment

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

"dividing the gCount" > "dividing gCount"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Tutorials/i2c.md Outdated

*/

for (var i=0;i<3;i++){
Copy link
Member

Choose a reason for hiding this comment

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

Comment: three iterations for the three coordinates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Tutorials/i2c.md Outdated

/*

The for loop is iterated 3 times, in order to extract the x,y,z values.
Copy link
Member

Choose a reason for hiding this comment

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

In my opinion, clearer to break this out into line by line comments in the code

Tutorials/i2c.md Outdated
var gCount=(dataReceived[i*2] << 8) | dataReceived[(i*2)+1];
gCount=gCount >> 4;

// 127 is checking whether we have a 0 or a 1 at the first position - basically its sign.
Copy link
Member

Choose a reason for hiding this comment

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

Can you give this a bit more explanation? Where does 127 come from, specifically? Tie back to 2's complement

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed

@mittalshravika
Copy link
Contributor Author

@Frijol, we have made all the changes you had requested. You may review the same.

Copy link
Member

@Frijol Frijol left a comment

Choose a reason for hiding this comment

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

All right, on to the more thorough critique ;)

@@ -0,0 +1,110 @@
# I2C

I2C is a simple-to-use communication protocol which can have more than one master, with the upper bus speed defined, and only two wires with pull-up resistors are needed to connect almost _unlimited_ number of I2C devices.
Copy link
Member

Choose a reason for hiding this comment

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

strike "simple-to-use"– that's a value judgment and might make some readers think along the lines of "well, I couldn't figure it out..". You can say it's "designed to be simple" if you want, but I think the rest of your description (two wires, unlimited devices) is more helpful


I2C is a simple-to-use communication protocol which can have more than one master, with the upper bus speed defined, and only two wires with pull-up resistors are needed to connect almost _unlimited_ number of I2C devices.

Each slave device has a unique address. Transfer from and to master device is serial and it is split into 8-bit packets. All these simple requirements make it very simple to implement I2C interface even with cheap microcontrollers that have no special I2C hardware controller. You only need 2 free I/O pins and few simple I2C routines to send and receive commands.
Copy link
Member

Choose a reason for hiding this comment

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

"Transfer from and to master device is serial and it is split into.." -> "Information transfer to and from the master device is serial. Data is split into 8-bit packets."


I2C is a simple-to-use communication protocol which can have more than one master, with the upper bus speed defined, and only two wires with pull-up resistors are needed to connect almost _unlimited_ number of I2C devices.

Each slave device has a unique address. Transfer from and to master device is serial and it is split into 8-bit packets. All these simple requirements make it very simple to implement I2C interface even with cheap microcontrollers that have no special I2C hardware controller. You only need 2 free I/O pins and few simple I2C routines to send and receive commands.
Copy link
Member

Choose a reason for hiding this comment

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

Second sentence uses "simple" twice but should ideally avoid it. Something more like "An I2C interface can be implemented even with cheap microcontrollers that have no special I2C hardware controller. The protocol only requires two I/O pins..."


I2C is a simple-to-use communication protocol which can have more than one master, with the upper bus speed defined, and only two wires with pull-up resistors are needed to connect almost _unlimited_ number of I2C devices.

Each slave device has a unique address. Transfer from and to master device is serial and it is split into 8-bit packets. All these simple requirements make it very simple to implement I2C interface even with cheap microcontrollers that have no special I2C hardware controller. You only need 2 free I/O pins and few simple I2C routines to send and receive commands.
Copy link
Member

Choose a reason for hiding this comment

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

"...a few simple I2C routines" – what's a more descriptive word than "simple"? Do you mean that they require few lines of code? That the logic is easily parsed? Is the descriptor helpful, or superfluous?


Each slave device has a unique address. Transfer from and to master device is serial and it is split into 8-bit packets. All these simple requirements make it very simple to implement I2C interface even with cheap microcontrollers that have no special I2C hardware controller. You only need 2 free I/O pins and few simple I2C routines to send and receive commands.

Let us get the accelerometer values using the I2C protocol. We will be using the [The Tessel Accelerometer](https://www.seeedstudio.com/Tessel-Accelerometer-Module-p-2223.html)
Copy link
Member

Choose a reason for hiding this comment

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

Beginning the phrasing with "In this tutorial..." and a description of the end result of the tutorial might be more clear here


The x,y, and z accelerometer values are stored in their respective OUT_MSB and OUT_LSB registers as negative values i.e. their 2's complement form.
Therefore in order to obtain the correct values, check whether the most significant bit of the OUT_MSB is 1 or 0 i.e whether the coordinate value is negative or positive.
If it is negative (checked using 0x7F, which is the maximum possible number that can be made from 7 bits), the if condition changes it to a 2's complement form, thus making it positive and adding a "-" sign in front of it.
Copy link
Member

Choose a reason for hiding this comment

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

Would it be clearer to use > 0b01111111 ?

### Sample Ouput

![output](https://i.imgur.com/Dg462Jf.jpg)

Copy link
Member

Choose a reason for hiding this comment

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

Great! Can you think of a bonus question to ask?


![output](https://i.imgur.com/Dg462Jf.jpg)

[Datasheet for accelerometer module](http://www.nxp.com/docs/en/data-sheet/MMA8452Q.pdf)
Copy link
Member

Choose a reason for hiding this comment

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

These seem like a separate section, not part of Sample Output


### Sample Ouput

![output](https://i.imgur.com/Dg462Jf.jpg)
Copy link
Member

Choose a reason for hiding this comment

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

This is your Fritzing diagram, not output

console.log('The x, y, z values are :',out);

});
```
Copy link
Member

Choose a reason for hiding this comment

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

Does your code actually work & respond appropriately when run? I think you're missing a crucial step– look up SYSMOD in the datasheet

@mittalshravika
Copy link
Contributor Author

Thanks a lot for the reviews!! Will make the changes shortly.

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.

4 participants