madison-embedded / gcc-builds

For projects built with the GNU toolchain.
GNU General Public License v3.0
11 stars 8 forks source link

STM32 Nucleo 144 F767ZI: Missing I2C Driver #1

Closed vkottler closed 7 years ago

vkottler commented 7 years ago

Need I2C implemented for various sensors and peripherals.

csgreen3 commented 7 years ago

What is the status of this driver? This needs go be done ASAP. If possible by the end of the work week.

chroro commented 7 years ago

We will try to finish today.

chroro commented 7 years ago

We are at the lab but we could not test because we could not find an available board. Do you have one so we can test the driver ?

csgreen3 commented 7 years ago

There should be 4 of them in a bag on the desk. We just ordered 4 more of them. When is the next time you can com in? I will try to be there.

chroro commented 7 years ago

Ill be at engineering hall all day.We couldnt get into a lab. Did you guys change the lock ? )

vkottler commented 7 years ago

Messaged you both on Slack about the code

davidguyuchen commented 7 years ago

I was working on it through the weekend, please let me know if I can help.

chroro commented 7 years ago

We are currently on a modification and testing stage. We will be at the lab tomorrow. Come join us.

vkottler commented 7 years ago

If we need any inspiration for function interface names etc. check out Driver_I2C.h and Driver_I2C.c.

Let's start to try and shoot for daily / more frequent commits. I know I saw you guys send data last night so I'm wondering how close we are to making a pull request

vkottler commented 7 years ago

Once I2C read and write is available we can then move on to writing individual device drivers, which will be a majority of the work.

vkottler commented 7 years ago

Hey guys, a couple of things:

Please do not commit changes to common/main.c. There should be no reason whatsoever to add initialization, remove post output and pretty much disable the command line interface. What was the motivation there?

All initialization can be added to common/nuc144.c.

You guys also don't seem to be aware that there are bit definitions in addition to struct definitions for each peripheral. I'm going to need every single hard-coded bit shift to be converted to it's defined value in stm32f767xx.h (gets included in proc/defs.h).

Right now I'm a bit frustrated because for any work to be done on the entire repository right now we have to undo everything you committed to main.

Tserendulam commented 7 years ago

I am sorry to change the main.c

We only commented out the original main so it is easy to fix it.

We can do it today.


From: Vaughn Kottler notifications@github.com Sent: Friday, July 28, 2017 11:53:03 AM To: madison-embedded/gcc-builds Cc: Tserendulam Ulam-orgikh; Assign Subject: Re: [madison-embedded/gcc-builds] STM32 Nucleo 144 F767ZI: Missing I2C Driver (#1)

Hey guys, a couple of things:

Please do not commit changes to common/main.chttps://github.com/madison-embedded/gcc-builds/blob/master/common/main.c. There should be no reason whatsoever to add initialization, remove post output and pretty much disable the command line interface. What was the motivation there?

All initialization can be added to common/nuc144.chttps://github.com/madison-embedded/gcc-builds/blob/master/common/nuc144.c.

You guys also don't seem to be aware that there are bit definitions in addition to struct definitions for each peripheral. I'm going to need every single hard-coded bit shift to be converted to it's defined value in stm32f767xx.hhttps://github.com/madison-embedded/gcc-builds/blob/master/proc/stm32f767zi/stm32f767xx.h (gets included in proc/defs.hhttps://github.com/madison-embedded/gcc-builds/blob/master/proc/stm32f767zi/defs.h).

Right now I'm a bit frustrated because for any work to be done on the entire repository right now we have to undo everything you committed to main.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/madison-embedded/gcc-builds/issues/1#issuecomment-318706061, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AZLTJdNWJF8DRnsHr29N6vpgm8JZb1wUks5sShHvgaJpZM4N7YX1.

vkottler commented 7 years ago

Awesome, no worries!

chroro commented 7 years ago

We forgot to undo the changes in main. But I'm sure it should not be hard to revert it since, all of your code is commented. " There should be no reason whatsoever to add initialization, remove post output and pretty much disable the command line interface. What was the motivation there ? " RE: We wanted to initialize and test the driver as soon as possible, so we did not even bother to read the main, so we just commented out the part we do not need.

" Right now I'm a bit frustrated because for any work to be done on the entire repository right now we have to undo everything you committed to main "

RE:Calm down. I believe reverting is not a big deal. As Tsereldulam said, we can do it today. It should be a single command.

vkottler commented 7 years ago

Calm down.

I'm sorry if you took some offense here.

chroro commented 7 years ago

No offense taken. Sorry if it sounded mean man, I did not intend it. We will change all the bit definitions and function names.

vkottler commented 7 years ago

All good man, as long as the interface is easy to use and works well we don't need to go deeply into making it more readable with the bit definitions, I didn't have good reason to mention that.

If you have time though that would be awesome (: we also need to use this interface to create device drivers for a pressure sensor an accelerometer which I will get working on issues for

chroro commented 7 years ago

Check it and comment it if something needs to be changed.

vkottler commented 7 years ago

What kind of buffer are you guys using?

I see volatile uint8_t* BUFFER_I2C = NULL; but I don't see any init function where that's malloc'ed

Thoughts on using a produce-consumer buffer? We use that for UART.

^ I see that the buffer is passed in with the function which makes sense

Do you guys have an example of how to do the initialization somewhere?

chroro commented 7 years ago

I'll be there at 6 pm and do the initializiation as well as MPU9250 driver test.

vkottler commented 7 years ago

Sounds good, check out the HAL drivers I added to the repo for inspiration

Their description on how to initialize their driver (might be similar for ours):

==============================================================================
                        ##### How to use this driver #####
  ==============================================================================
    [..]
    The I2C HAL driver can be used as follows:
    (#) Declare a I2C_HandleTypeDef handle structure, for example:
        I2C_HandleTypeDef  hi2c;
    (#)Initialize the I2C low level resources by implementing the HAL_I2C_MspInit() API:
        (##) Enable the I2Cx interface clock
        (##) I2C pins configuration
            (+++) Enable the clock for the I2C GPIOs
            (+++) Configure I2C pins as alternate function open-drain
        (##) NVIC configuration if you need to use interrupt process
            (+++) Configure the I2Cx interrupt priority
            (+++) Enable the NVIC I2C IRQ Channel
        (##) DMA Configuration if you need to use DMA process
            (+++) Declare a DMA_HandleTypeDef handle structure for the transmit or receive stream
            (+++) Enable the DMAx interface clock using
            (+++) Configure the DMA handle parameters
            (+++) Configure the DMA Tx or Rx stream
            (+++) Associate the initialized DMA handle to the hi2c DMA Tx or Rx handle
            (+++) Configure the priority and enable the NVIC for the transfer complete interrupt on
                  the DMA Tx or Rx stream
    (#) Configure the Communication Clock Timing, Own Address1, Master Addressing mode, Dual Addressing mode,
        Own Address2, Own Address2 Mask, General call and Nostretch mode in the hi2c Init structure.
    (#) Initialize the I2C registers by calling the HAL_I2C_Init(), configures also the low level Hardware
        (GPIO, CLOCK, NVIC...etc) by calling the customized HAL_I2C_MspInit(&hi2c) API.
    (#) To check if target device is ready for communication, use the function HAL_I2C_IsDeviceReady()
vkottler commented 7 years ago
 I2C_init(I2C2_BASE);
    /* MPU9250  */
    initAK8963(dest,I2C2_BASE);
    initMPU9250(I2C2_BASE);
    MPU9250SelfTest(dest);
    printf("MPU9250 deviation = %f\r\n", *dest);    

    /* Should be 0x71 otherwise MPU9250 not working properly */
    if ( 0x71 !=  readByte(0x68, WHO_AM_I_MPU9250))
    {
        printf("MPU9250 Fail\r\n");
        return false;
    }

What is the behavior if the device is not attached? Can we make get a function going to "query" an I2C address and check for ACK?

Also what is all of the code for the AK8963 for?

vkottler commented 7 years ago

Any idea when this might be finished and some of the above concerns will be addressed?

Should we just use the HAL driver? A few others need this code finalized to begin working on device drivers

Tserendulam commented 7 years ago

We could add some function to check if a device is attached tomorrow. What are the other concerns ? as far as I know we addressed above issues; for example, you can see initialization in above comment.All you need to pass is which I2C Base for now assuming timing and size of address is default.

vkottler commented 7 years ago
bool I2C_setSCLDEL(uint8_t scldel, uint32_t i2c_base) 
{

    I2C_TypeDef * i2c = ( I2C_TypeDef * ) i2c_base;

Why not just make I2C_TypeDef *interface the function argument? Also why so many newlines? Can you refactor to follow style paradigms in the rest of the codebase?

Tserendulam commented 7 years ago

I think it's a matter of style. I learned this style from ECE353. Furthermore, you did not provide any specifications beforehand.

csgreen3 commented 7 years ago

It seems like the best option with the amount of time we have left it to start porting over the HAL driver. Thoughts?

Tserendulam commented 7 years ago

It's up to you guys decision. However, as we said earlier, the driver is done except checking if a device is attached.

vkottler commented 7 years ago

that's good to hear. Do you have any sample console output by chance and is it possible to use this driver at runtime via a command?

Tserendulam commented 7 years ago

If you make install the chroro branch with MPU9250 setup, you can see the console output of MPU9250.

You cannot use it through command yet. as we did not add it.


From: Vaughn Kottler notifications@github.com Sent: Monday, August 7, 2017 1:03:51 PM To: madison-embedded/gcc-builds Cc: Tserendulam Ulam-orgikh; Assign Subject: Re: [madison-embedded/gcc-builds] STM32 Nucleo 144 F767ZI: Missing I2C Driver (#1)

that's good to hear. Do you have any sample console output by chance and is it possible to use this driver at runtime via a command?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/madison-embedded/gcc-builds/issues/1#issuecomment-320736587, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AZLTJbc695s_2-XVnyXdIzqKexDDPTJtks5sV1GHgaJpZM4N7YX1.

csgreen3 commented 7 years ago

Sounds good I will be working on getting all the I2C sensors working so if anyone has time to come in it would be awesome

chroro commented 7 years ago

Sorry, I'm going back to my country soon, and I have been busy. Do you still need help ? I can come tonight.

csgreen3 commented 7 years ago

that ok I think we got it working Example i2c command => i2c 66: TIMEOUT 67: TIMEOUT 68: OK 69: TIMEOUT 6a: TIMEOUT 6b: TIMEOUT 6c: TIMEOUT 6d: TIMEOUT 6e: TIMEOUT 6f: TIMEOUT 70: TIMEOUT 71: TIMEOUT 72: TIMEOUT 73: TIMEOUT 74: TIMEOUT 75: TIMEOUT 76: TIMEOUT 77: TIMEOUT 78: TIMEOUT 79: TIMEOUT 7a: TIMEOUT 7b: TIMEOUT 7c: TIMEOUT 7d: TIMEOUT 7e: TIMEOUT 7f: TIMEOUT => i2c read 68 117 OK address: 0x68 Data: 0x71 => i2c read 68 49 OK address: 0x68 Data: 0x0 => i2c write 68 49 ff OK => i2c read 68 49 OK address: 0x68 Data: 0xff =>

Tserendulam commented 7 years ago

sounds good. Are you using HAL driver?

csgreen3 commented 7 years ago

Yeah sorry. It wasn't that your driver didn't work it was the the functionality of the HAL driver

vkottler commented 7 years ago

Since we're using HAL I'm closing this