Rate my code

[lawrence11]

So whats the solution?

Delete them?

[jbee]

So whats the solution?

Delete them?

If you're planning on using the union, then yes.  But if that's all you do, it won't compile.  You need to declare your union inside main like

Code: [Select]

union {
uint16_t prebufferk[4];
uint8_t bufferk[8];
} combok;

Here you're defining combok as a union of those two objects.  Then when you want to set/read those prebufferk/bufferk variables, you do combok.prebufferk = foo;  Since prebufferk and bufferk are members of that union, you need to use the period operator to access them.

From what I can tell it looks like you're trying to read a 16 bit value off the SPI data register and just echo it out of a USB interface that takes 8 bit values?

What's the point of doing all this

Code: [Select]

  /* USER CODE BEGIN 2 */
  bufferk[0]=1;//reportID
  bufferk[1]=0;//modifier
  bufferk[2]=0;//OEM
  bufferk[3]=0;//keycode data
  bufferk[4]=0;//keycode data
  bufferk[5]=0;//keycode data
  bufferk[6]=0;//keycode data
  bufferk[7]=0;//keycode data

If when the first call to prebufferk[index_k]=SPI2->DR; just overwrites that data?

[lawrence11]

Yes it overwrite.

That data is remnant, should be all 0.

Its just a SPI to USB passover.

Functions that write to usb take uint_8  as parameter, while SPI read register can be read in 16 bit instructions.

Thus the need to unionize, it will take the raw binary and pass it.

Consideration for what it is is the job of SPI transmitter.

[lawrence11]

jbee is the wise one.

All you complainers overlooked a massive flaw.

Frikkin just jackoff in my threads and belittle the self learnr all the time.

[jbee]

You can avoid the union if you just have the uint16_t data type (let's keep it prebufferk), and when you call USBD_HID_SendReport(&hUsbDeviceHS, bufferk, 8 );, you cast it to a uint8_t * like someone else pointed out earlier in the thread.  Since it looks like the call takes a pointer to the data buffer and number of bytes, you could just do:

Code: [Select]

USBD_HID_SendReport(&hUsbDeviceHS, (uint8_t *)prebufferk, 8);

Assuming the part where you're reading from the SPI register and indexing is working correctly, this should work fine.  And even better:

Code: [Select]

USBD_HID_SendReport(&hUsbDeviceHS, (uint8_t *)prebufferk, sizeof(prebufferk));

When you pass a pointer to a function, you're essentially passing the memory address of the first element in the allocated memory block.  You've allocated 4 uint16_t's in memory (uint16_t prebufferk[4]; ).  Which is the same as 8 uint8_t's if you envision it in the memory stack.  You can safely cast it to a uint8_t * and tell the function to read out 8 bytes.

You use sizeof() so if you change the size of the prebufferk and all the calls still work; you don't have to go through everything and change the "8" to something else.  And again, since prebufferk is an array of 4 uint16_t's, sizeof(prebufferk) is 2 bytes * 4 elements = 8.

[lawrence11]

So the question is, what the hell is union good for anyways and does it result in the same?

[jbee]

I mean, what you were initially trying to do would have most likely worked (I don't like to say for sure unless I can test it), but in your case, when passing a pointer to a function that simply has a different type, a cast is what is used.

I've never really sat down to think about when to use unions.  I've been doing this for a while, so you just kind of know when to use a union from experience.  An example is I want to read from EEPROM.  My EEPROMREAD function only returns a byte, but I have a 16-bit checksum in there I want to get.  so I can do

Code: [Select]

union {
  uint16_t chksum;
  uint8_t in[2];
} myUnion;

myUnion.in[0] = EEPROMREAD(chksumAddress);
myUnion.in[1] = EEPROMREAD(chksumAddress + 1);

Now my 16-bit checksum is in myUnion.chksum and I didn't have to do any crazy, unreadable pointer arithmetic or anything (if it's even possible, I can't even think of a way in this situation).

There are much better answers here.  If I attempted to explain, I would just reiterate the answers only more poorly.

https://stackoverflow.com/questions/252552/why-do-we-need-c-unions

[lawrence11]

So I was probing around, to see if my transmit chip sends the voltage to the other pin, across this massive 6 layer PCB I made.

I see that when GPIO is set, one MOSI is at 2 volts, just above the "legal limit" for a high value, and the SCLK is at the expected 3.3 volts.

The pins in question are setup as SPI receive only.

Is this worrying? I am measuring this with a multimeter with long leads, I was in voltmeter ofc.

I can reset and switch it off at will, did I mess up?

[SiliconWizard]

So I was probing around, to see if my transmit chip sends the voltage to the other pin, across this massive 6 layer PCB I made.

I see that when GPIO is set, one MOSI is at 2 volts, just above the "legal limit" for a high value, and the SCLK is at the expected 3.3 volts.

The pins in question are setup as SPI receive only.

I don't know about your whole IO setup, but I think by default, if you're setting SPI receive only mode, the MOSI pin will not be driven by the SPI module and thus will be high-impedance (thus the 2V), unless you specifically set it as a GPIO output, or as an input with either pull-up or pull-down enabled.

[lawrence11]

So everything still unbroken yet?

One chip is set in SPI receive, while with the other, I make those "sending pins" into regular GPIO just to test.

I dont wanna send edges into long leads so I measure after the fact, I measured 2 volts and 3.27 volts

on those 2 pins.

This is also to confirm a change, when I order something in SW, that I indeed see a voltage lvl change on a pin, under a 200 pin BGA. I knew it was good for communicating with SWD and not blowing up.

Both were setup as push pull gpio output low speed.

I havent pulled the trigger yet on a scope, I have my 1984 tek at my mothers house, still dusty and in the closet, the problem is I just dont got space for it here on a table in a corner my girlfriend made for me.

[lawrence11]

Heres my quality check for measuring BGA installs, and other MCU installs.

After soldering the board and everything, you can understand that damaging the BGA would basically ruin my day and week and cause immediate stress on my weak heart.

[SiliconWizard]

Didn't realize this is a BGA. If you're certain about your IO configuration, then it may just be a matter of a bad ball connection...

[lawrence11]

seems unlikely, I measure, when it was all powered off.

from VDD rail, using right multimeter polarity, 30k ohms on this pin, and many other pins have 27k-29k 30k ohms, some have 10k ohm.

Anyways, I guess its good enough, 2 volts is higher than 1.888 and you need 1.888 to register high when VDD is 3.3 volts.

[SiliconWizard]

Still looks fishy.

I would write a very small test program in which you only set up ALL IOs as digital outputs, and test it this way...

[lawrence11]

So I'll answer my own question.

I was in fact measuring wrong and causing bus contention situation.

Since my board has no test points, I measure on the pin, and its difficult to measure a single pin, on the chip or on the pcb pin pad. My measurement caused the bad reading.

I tried measuring on a more remote via on the underside, pressed the tip hard and twisted bad and forth for like 30 secs. behold 3.3 volts

Pin next to it was set 0, it was itself a GPIO output PP.

[donotdespisethesnake]

Though, as you progress as a software developer, you'll find it necessary to develop a certain consistency and you'll learn that it really helps to place "hints" in the code for your brain to parse it more quickly. Discerning strictly between function calls and structural and control elements is one such hint that helps you along.

Being doing software over 30 years. What I learned is that there is a lot of bullshit like this around and it makes no difference to code quality. If coders really can't tell the difference between a function call and if statement without extra help then they are just shit coders, full stop.

The fact that coding standards are bullshit is proved by the fact every company has a different one. Learning to code based on the rules of one company just creates incompetent coders in my experience. So often have I reviewed code which follows the company guidelines perfectly, but is still full of bugs.

There does seem to be a correlation between slavish adherence to "rules" and bad programming. I think people who lack the knowledge of good programming substitute for that by making their code "look pretty".

[thinkfat]

Though, as you progress as a software developer, you'll find it necessary to develop a certain consistency and you'll learn that it really helps to place "hints" in the code for your brain to parse it more quickly. Discerning strictly between function calls and structural and control elements is one such hint that helps you along.

Being doing software over 30 years. What I learned is that there is a lot of bullshit like this around and it makes no difference to code quality. If coders really can't tell the difference between a function call and if statement without extra help then they are just shit coders, full stop.

The fact that coding standards are bullshit is proved by the fact every company has a different one. Learning to code based on the rules of one company just creates incompetent coders in my experience. So often have I reviewed code which follows the company guidelines perfectly, but is still full of bugs.

There does seem to be a correlation between slavish adherence to "rules" and bad programming. I think people who lack the knowledge of good programming substitute for that by making their code "look pretty".

Not developing a sense of order in your coding in such a long time is to me a sign of a sloppiness, if not a perturbed mind. And on the merit of coding rules, if you work in a team where more than a handful of developers work on the same code base, it definitely helps, no, it's a necessity to have common rules at least for style, if not for patterns as well.

[lawrence11]

I'm trying to not go the "global way" when mixig variables that get seen by IRQhandlers.

trying to segment things and limit the scope of things.

For instance, trying to make this variable spi1_tx_cnt a "static volatile".

I tried volatile, static, volatile static, nothing seems to work.

The xx_it.c file doesnt want nothing to do with it.

What line of code should I add in main.h? If any.

[thinkfat]

I'm trying to not go the "global way" when mixig variables that get seen by IRQhandlers.

trying to segment things and limit the scope of things.

For instance, trying to make this variable spi1_tx_cnt a "static volatile".

I tried volatile, static, volatile static, nothing seems to work.

The xx_it.c file doesnt want nothing to do with it.

What line of code should I add in main.h? If any.

spi1_tx_cnt can't be static because you need it in two compilation units. So it has to be global.

Put the following line in main.h:

Code: [Select]

extern volatile uint8_t spi_tx_cnt;
Some will frown over the "extern" and call it "superfluous". But it makes clear that this is a declaration, not a definition. It's one of those "style" things.

[lawrence11]

Ok, so I managed to make it compile with various combinations.
Wich is the best and what I want, in terms of limiting scope?

[thinkfat]

First version doesn't work since both compilation units will use their own definition of the variable. They are independent, no communication will happen.
Second version looks OK'ish, but you need to declare the variable in the interrupt handler file as "extern volatile". This version is OK from a isolation point of view.
Third version - looks fishy. You should initialize a variable where it's defined, not where it's declared. I wonder why the compiler doesn't complain here. Maybe it's "legal" from a C point of view but it's not good style.

[lawrence11]

so this here compiles.

in main.c

volatile uint8_t spi1_tx_cnt=0;

in xx_it.c

extern volatile uint8_t spi1_tx_cnt;

This the proper technique, no need static anywhere??

[thinkfat]

It's the only way that works, if you want to access the same variable in two compilation units.
You cannot use "static" because that will make the variable private to the compilation unit it is defined in. This is not what you want here.

Note: "compilation unit": all source and header files read by one compiler invocation. Typically, each source file with its include file tree form a separate compilation unit.

[SiliconWizard]

so this here compiles.

in main.c

volatile uint8_t spi1_tx_cnt=0;

in xx_it.c

extern volatile uint8_t spi1_tx_cnt;

This the proper technique, no need static anywhere??

This would be right indeed.

Don't use a static qualifier on declarations of variables that need to be visible across compilation units ("source files" if you wish). "static" is exactly meant for the opposite case: make some declaration invisible to any other compilation unit. To be used for declarations that are strictly confined to one compilation unit, and in this case, use it liberally. Also use it on function declarations (that again are not used outside of a given compilation unit), which is good practice.

An alternative: you could make your variable static
"static volatile uint8_t spi1_tx_cnt=0;"

and define an "accessor" function in the same compilation unit to access it from the outside, like:

uint8_t GetSPI1TxCnt(void)
{
    return spi1_tx_cnt;
}

Probably overkill for your example (and will add a small overhead), but this is also a common method for accessing variables outside of their compilation unit that gives a few benefits.

[lawrence11]

Ok thanks, this is confusing.

Static, extern, volatile. Always mixing these up.