SpenceKonde / ATTinyCore

Arduino core for ATtiny 1634, 828, x313, x4, x41, x5, x61, x7 and x8
Other
1.59k stars 308 forks source link

ATtiny1634 and MCP2515 #196

Closed alessio31183 closed 6 years ago

alessio31183 commented 6 years ago

Dear all, I'm trying to make the "receive_check" CAN_BUS example working on my attiny1634. What I did is to transform an UNO board replacing the ATmega328p with the ATtiny1634 on the DIP socket. I upload the bootloader correctly and I'm able to use the serial connection to upload any sketch. So I'm sure that the SPI is correctly connected to the uC. Setup is 16MHz overclocked in order to use the on board arduino UNO crystal. I wish to get data from the CANBUS using the seed studio can bus shield. The issue is that the simple "receive_check" doesn't work on my UNOtoATtiny board converted. Everything works with the normal UNO board.

I've switched the MISO & MOSI pins over because the Attiny1634 use te DI and DO pins to make the SPI protocol.

Looking at SCK signal, it seems to work correctly, even changing the clock speed. CS (chip select) is also OK. Even the MOSI seems to write something, but with a different value compared to what the UNO (atmega328p) writes. No reply from the MISO pin after the CAN.begin(CAN_250KBPS) function

Any suggestion?

ATtiny1634-to-UNO.pdf

SpenceKonde commented 6 years ago

What SPI mode does this use?

Can you post a trace of MOSI with Uno and with 1634? (ideally, a trace of SCK and MOSI on the same screen) You say you've found that they are outputting different data when they should be outputting the same thing - the most useful source of debugging information will likely be comparing the two outputs.

Would also need to see the code you're using.

alessio31183 commented 6 years ago

SPI mode should be the mode 0 as specified in the mcp_can.cpp file

define spi_readwrite SPI.transfer

define spi_read() spi_readwrite(0x00)

define SPI_BEGIN() SPI.beginTransaction(SPISettings(10000000, MSBFIRST, SPI_MODE0))

define SPI_END() SPI.endTransaction()

receive_check.txt

BELOW THE MOSI and CLOCK signal

ATMEGA328p atmega328p

ATTINY1634 attiny1634

SpenceKonde commented 6 years ago

Oh - are you using the latest version of the core (ie, pull from github)? There are a bunch of SPI fixes in it that aren't in the board manager version yet. If you're using 1.1.4, please uninstall it and do a github install, and see if that fixes it.

alessio31183 commented 6 years ago

Yes I've pulled the latest version from the github with the more B.O.D. settings just to figure out

This is the library that I'm using with the example where I've just changed the CS (pin2 instead of 10 and speed @ 250kbps) CAN_BUS_Shield.zip

SpenceKonde commented 6 years ago

wtf.... @exzombie - this looks bad - what do you think? (tagging since you dug deeper into this code than I did). I was just about to do a release today, but this looks like a show stopper.

It looks like the data output is inverted.... @alessio31183 - can you confirm that those traces are labled the right way around, ie, the one where MOSI is idle high is the Uno, not the tiny1634? I thought MOSI was normally held low while idle (as shown on lower trace)

exzombie commented 6 years ago

@SpenceKonde agreed, SPI_MODE0 should be low while idle. Actually, of all the modes, MODE0 and MSBFIRST combination is the only one I could test with an actual device, and in fact, I'm using it as I write this. I'll bring out a logic analyzer later tonight just to confirm, but it seems to me that the atmega trace is wrong :/

SpenceKonde commented 6 years ago

It's trying to clock out a 0xC0; The lower trace looks right, not the upper trace!

exzombie commented 6 years ago

@alessio31183 - Are you completely sure the two traces are supposed to be of the same data?

@SpenceKonde - Where do you see that they should to be sending 0x0C? I traced my UNO and an ATtiny84 with a logic analyzer and they match the images above with 0x02 and 0x05 respectively. To be clear, I actually tell them to send these bytes. If I program the ATtiny84 to send 0x02, it will not look like the image above. So, works for me.

I remembered that the ATmega hardware SPI seems to keep MOSI high when idle. I chose to keep it low on ATtiny, since it shouldn't matter which it is as long as CS is not asserted. Is there a power consuption concern? If there are pullups on data lines, it makes sense to keep them high when not in use, but I don't know what the practice is.

UNO sending 0x02

spi_uno_0x02

ATtiny84 sending 0x05

spi_attiny84_0x05

Sketch

#include "SPI.h"

void setup() {
  pinMode(SS, OUTPUT);
  digitalWrite(SS, HIGH);
  SPI.begin();
}

void loop() {
  delay(100);
  SPI.beginTransaction(SPISettings(10000000, MSBFIRST, SPI_MODE0));
  digitalWrite(SS, LOW);
  SPI.transfer(0x05);
  digitalWrite(SS, HIGH);
  SPI.endTransaction();
}
SpenceKonde commented 6 years ago

Okay, actually I'm not sure about my claim about what byte it's writing now.

@alessio31183 , what byte is it supposed to be writing in those examples? I thought it was the MCP reset instruction, but that would be what we'd be getting if doing LSBFIRST - so nevermind. I've got nothing.

alessio31183 commented 6 years ago

I've tried to upload the test_SPI sketch writing 0x05 on it. The only different is the idle state that in the UNO is "HIGH" differently on the ATtiny1634 that is "LOW" But this should not be an issue. I'm thinking that probably there is something strange in the mcp_can library. But let's try to start from the beginning. Which different there are in the "receive_check" example on atmega328p and on the attiny1634? For me no differences, so I supposed that this sketch should work on the attiny as well.

Trying with this sketch instead:

include

include "mcp_can.h"

// the cs pin of the version after v1.1 is default to D9 // v0.9b and v1.0 is default D10 const int SPI_CS_PIN = 2;

MCP_CAN CAN(SPI_CS_PIN); // Set CS pin

void setup() { Serial.begin(115200);
Serial.println("CAN BUS Shield init");

}

void loop() { if (CAN_OK != CAN.begin(CAN_250KBPS)) Serial.println("CAN BUS Shield init fail"); else Serial.println("CAN BUS Shield init ok!");

delay(1000); }

I'm not able to get any reply on MISO from the mcp2515. Pin always "LOW"

The SPI seems to work correctly.

alessio31183 commented 6 years ago

On 06/03/2018 13:45, Jure Varlec wrote:

|DEBUG_MODE| to 1

Already did it with no more information.

I've got "Enter setting mode fail" that means that the first set of commands:

0x05, 0x0F, 0xE0, 0x80 are not interpreted from the mcp2515 correctly

exzombie commented 6 years ago

Dang, I forget to refresh the page, and we get a race condition editing our posts :) .

So are you using this library? Looking at the code, it does not seem to use the beginTransaction() API. In the last sketch you posted that calls CAN.begin() in the loop, please put SPI.setClockDivider(SPI_CLOCK_DIV2) into setup(). Then please also try SPI_CLOCK_DIV4, SPI_CLOCK_DIV8 and SPI_CLOCK_DIV32, the code could do with some exercise.

alessio31183 commented 6 years ago

@exzombie I'm not using that library. Mine is attached in a previous post.

Yesterday I've tried changing the SPI speed with no success.

exzombie commented 6 years ago

Ah, that one is almost the same, except it does use transactions, so it should work fine. Also, the MOSI traces you posted look correct. I can't come up with anything else, and without either the same uC or the same slave, I can't try things on my own :(

Maybe you could try this library? It's by the same author as the LoRa library I'm using. Maybe it working or not working will give us a clue.

alessio31183 commented 6 years ago

Uploading CANReceiver example and changing the

define MCP2515_DEFAULT_CS_PIN 2 // for attiny1634

define MCP2515_DEFAULT_INT_PIN 11 // for attiny 1634

I got on serial:

CAN Receiver Starting CAN failed!

Using Arduino UNO it works

exzombie commented 6 years ago

@SpenceKonde - the version of the CAN library I was looking at that didn't use transactions, only called SPI.begin() and nothing else. In the latest tinySPI, this only sets up pin directions and USI for mode 0, but does not select the clock. The reason for this is code size; I never considered that SPI would be used without specifying its parameters, and indeed a user would quickly find that he has to do that. But when using a broken library, the user can't know why it fails without looking at the code.

So we have the following options:

  1. do nothing, keep the code small
  2. choose a default clock rate that will always be compiled in
    1. choose the smallest function (freq <= F_CPU/14, 26 instructions)
    2. choose the fastest function (freq = F_CPU/2, 86 instructions)
    3. compromise (freq = F_CPU/4, 40 instructions)

Note that the code size increase would be in addition to what is actually used. At least the examples I saw used 10Mhz clocking, which I guess would always yield F_CPU/2, being faster than most of these microcontrollers.

alessio31183 commented 6 years ago

@exzombie if I change the mcp_can.cpp file like this

define SPI_BEGIN() SPI.begin()// SPI.beginTransaction(SPISettings(10000000, MSBFIRST, SPI_MODE0))

So without SPI specifications, the UNO works anyway @ 4MHz (F_CPU/8) No SPI is working using the ATtiny because the SPI setting must be specified

Using 10Mhz in SPISettings the SPI clock is setup at maximum speed (8MHz)

exzombie commented 6 years ago

Because an ATmega chip cannot have an undefined SPI clock, and the defaults work for most slaves. But only using begin() is broken, and in fact the whole old non-transactional API is wrong since it can cause problems with interrupt routines and such. And that is why your version of the library was updated to use transactions, from the old version that I found on Github.

Anyway, this initialization issue is only tangetial to this discussion, because this is not the problem you have. I only described it to @SpenceKonde because we will need to decide on what to do.

alessio31183 commented 6 years ago

@exzombie yes I had got your clarification.

Anyway, my modest opinion is that the SPI writing process is right, probably there is something in the receive procedure.

Some time ago, in another project which was using the same uC, I had made the SPI communication by me, without using the SPI (tinySPI) library and the USI peripheral.

I had used the USART in SPI mode in order to have the speed @ FCPU/2 before the last ATTinyCore version.

These were the function created:

void spi_config(unsigned char master, unsigned char mode, unsigned char speed) { DDRA |= (1<<PA6); // set PA6 as output PORTA |= (1<<PA6); // set PA6 HIGH UBRR1 = 0; XCK1_DDR |= (1<<XCK1); //SCK UCSR1C = (3<<UMSEL10)|mode; UCSR1B = (1<<RXEN1)|(1<<TXEN1); UBRR1 = speed; }

unsigned char spi(unsigned char dat) { while ((UCSR1A & (1<<UDRE1)) == 0) ; UDR1 = dat; while ((UCSR1A & (1<<RXC1)) == 0) ; return UDR1; }

void writeReg(uint8_t reg, uint8_t value) { PORTA &= ~(1<<PA6); // chip select LOW state spi(reg & 0x7F); spi(value); PORTA |= (1<<PA6); // chip select HIGH state }

uint8_t readReg(uint8_t reg) { uint8_t data; PORTA &= ~(1<<PA6); // chip select LOW state spi(reg | 0x80); // bit 0: READ bit. The value is 1. data = spi(DUMMY); PORTA |= (1<<PA6); // chip select HIGH state return data; }

Calling the spi_config like this: spi_config(1, 3, 0); // MASTER, mode 3, F_CPU/2 I was able to use the SPI @ FCPU/2 (4MHz with the internal 8MHz oscillator)

Hope this can be useful

exzombie commented 6 years ago

Hope this can be useful

Sadly, no :( . The tinySPI implemetation does not use USART, but USI. And USI is symmetrical wrt sending and receiving, so if data is going out, it should also be coming in, if the slave would, in fact, send someting.

So the MOSI and CLK lines act as they should, as evidenced by your capture. The MISO line stays low. I see three options:

  1. there is something wrong with the CS line
  2. something is driving MISO low
  3. while MOSI and CLK look right to the eye, there is something subtly wrong and the slave doesn't receive

I don't like option 3. much because I can't help debug it, I don't have the hardware. Option 1. is the easiest, so please first check that the CS line is correctly asserted, maybe with a trace capture along with CLK. As for option 2., all I can do is check in the code that pin numbers and registers are correct, and for all I can see in pins_arduino.h and the datasheet, they are :/ . So if it's option 2., it must be something external to the SPI code, because it correctly sets MISO to input mode. But maybe you can manually set pinMode to INPUT and disable the internal pullup, just to check?

alessio31183 commented 6 years ago

The tinySPI implemetation does not use USART, but USI

As I expected :(

  1. there is something wrong with the CS line

Checked at the beginning and the behavior is the same as UNO (CS low when MOSI speaks)

exzombie commented 6 years ago

I just reread the whole discussion and as far as I can see, there are three things left to try. There's trying pinMode and digitalWrite to set up MISO, because these functions do more than just set PORTx and DDRx.

Then there's testing another CAN library, just to see if it makes a difference. Even if it works, this is still a bug (compatibility with UNO is kinda important, I think), but it may help narrow it down.

And the last thing (which should probably be first) is testing commit c4f9aa89ea11740775a0c24c85f4898255e5bda5, which is before the big USI change. This will tell us whether we are dealing with a regression.

Apart from that, I'm out of ideas.

SpenceKonde commented 6 years ago

@exzombie - IMO - re: default clock, put the smallest option in as default - if they're not bothering to specify a speed, they obviously don't care about the details of the SPI configuration, and when in doubt,m bet Where does this change need to be made? Do you want me to do it or just PR it yourself? (also - as an aside - have I already mailed you some freebies? If not, email me your mailing address)

I've changed this issue from "bug" to "mystery" since things seem to be getting murkier.

exzombie commented 6 years ago

@SpenceKonde - Yeah, that's why I already used F_CPU/16 as the default for SPISettings. Most libraries will pull in all of the code anyway, along with 32-bit math :( . I'll make a PR, but I noticed that there may be an additional problem with modes other than MODE0 and I'd like to check that first. Lack of devices is holding me back, I hope to find some time to turn an UNO into a slave for testing. By next week, at the latest; I won't have much free time after that.

alessio31183 commented 6 years ago

Dear all, first of all thanks to all for the support. I'm so sorry because at the end the issue was on my canBUS shield. I though that the shield was connected to the Arduino trhough the pin 13, 12, 11 and 10 (CS) but going deeper on the PCB I found that the connection was on the ICSP connector only.

So I solved switching the mcp2515's SO and SI over the component directly, not as before on the Arduino pinout. For anyone that want to use the seedstudio CAN-BUS shield 1,2 remember to switch the SO and SI on the component using the USI interface.

Now the attiny1634 is able to speak with the CAN controller correctly.

The SPI library is absolutely correct as it is in the 1.1.5 version.

exzombie commented 6 years ago

@alessio31183 - hey, no worries, glad you solved the mystery. I need to keep this issue in mind, as I plan to misuse the ICSP port myself :/ . I'm glad to see the code got some more testing before being released, and we did find (and fix) a real compatibility issue, thanks to you poking at it.

SpenceKonde commented 6 years ago

Mystery solved!

Glad we caught that other issue, too.

Re: using ICSP header for SPI - that is the recommended way to get on the SPI pins, all well designed shields will do that; the reason being that on the Mega, the SPI pins are not pins 11/12/13, and are in a different place on the board. To ensure compatibility with Mega, Uno and Leo with a single shield, you use the ICSP header to get at the SPI pins, which will always have the SPI pins on it.