Jump to content
 

holdmybeer

Member
  • Content Count

    30
  • Joined

  • Last visited

  • Days Won

    6

holdmybeer last won the day on September 4

holdmybeer had the most liked content!

Community Reputation

8 Neutral

About holdmybeer

  • Rank
    Pupil

Recent Profile Visitors

The recent visitors block is disabled and is not being shown to other users.

  1. I agree with you complaints. You can mitigate the probability of making errors, but you can't get it to zero. 1. True, you have to continue typing to at least SERCOM_[register]_ to get more meaningful suggestions. The good thing is, there is a pattern in all header files. In doubt, follow to the definition in the header files, they are relatively small. 2. It will compile, as any magical number will compile. But you will at least have a chance to find a suspicious looking symbol by carefully reading through the statement. 3. This is done much better with the new DFPs. Have a close look in the XC32 compiler directory. You'll find an /include directory AND an /include_mcc! The first directory is the legacy code. By default, the new definitions will be used. 4. Similar to 2. 5. That is a great point, it is indeed not MISRA compliant. I'd guess that an exception is made then, but it would be interesting how this is handled in larger companies. Don't mix up Atmel START (ASF libraries) with DFPs. I personally don't like to use START, too much layer-over-layer. If you use it, you have to fully rely on the code underneath all the layers. And often enough, there is a bug hidden in some hri function. At this point, ASF doesn't save time anymore.
  2. Well, yes. My code example didn't show the WHY, because the point was to discuss about the avoidance of magic numbers as a mean to be an end in itself. This is non-exclusive: void UART_init(void) { // Initialize SERCOM link // for details, see datasheet (DS60001507E) p. 944, 34.8.1 // use DORD to change bit order, choose pins with RXPO/TXPO SERCOM4_REGS->USART_INT.SERCOM_CTRLA = SERCOM_USART_INT_CTRLA_MODE(1) | SERCOM_USART_INT_CTRLA_RXPO(3) | SERCOM_USART_INT_CTRLA_TXPO(1) | SERCOM_USART_INT_CTRLA_DORD(1); } Since this is mostly precompiler stuff, no speed or code size is sacrificed. DFPs help me to write human-readable register initializations without making a calculation error in my mind. If someone else wants to change this later (switch RXPO for example), he doesn't have to decipher the actual value of RXPO from my magic value (first source of error) and doesn't have to recipher it into a new magic value (second source). This "someone else" could also be me after having an argument with my project leader. And finally I don't need to maintain clarifying comments. I could read the statement aloud and would be still comprehensible what I'm trying to accomplish. The IDE also helps a lot with getting the right symbols. The XC32 team has done a nice job having a logic in the names, so if I start typing "SERCOM_UART_" and hit <CTRL> - <SPACE>, I'll get a selection which fits, so I don't need to remember the exact name. If I'm unsure if this is the right value, I just hold <CTRL> and left click on the symbol. MPLABX then jumps right into the include header, letting me verify my decision: #define SERCOM_USART_INT_CTRLA_RXPO_Pos _U_(20) /**< (SERCOM_USART_INT_CTRLA) Receive Data Pinout Position */ #define SERCOM_USART_INT_CTRLA_RXPO_Msk (_U_(0x3) << SERCOM_USART_INT_CTRLA_RXPO_Pos) /**< (SERCOM_USART_INT_CTRLA) Receive Data Pinout Mask */ #define SERCOM_USART_INT_CTRLA_RXPO(value) (SERCOM_USART_INT_CTRLA_RXPO_Msk & ((value) << SERCOM_USART_INT_CTRLA_RXPO_Pos))
  3. I wouldn't duplicate the datasheet either. But isn't describing the calculation already the same? To clarify, both suggestions in an example: Describe the calculation: // comparator configuration // bit 6 = C1OUT, bit 4 = C1INV => 0101 // bit 3 = CIS, switch to RA0/AN0 // CM<2:0> = 101, one independent comparator CMCON0 = 0x5d; Describe the register: // comparator configuration // bit 7 = C2OUT // bit 6 = C1OUT // bit 5 = C2INV // bit 4 = C1INV // bit 3 = CIS // bit <2:0> = comparator mode CMCON0 = 0x5d; The latter has the advantage of not getting out of sync, but is a tedious and error-prone work. And if you do the work, why not describe it with #define and put into a header file to re-use it for other projects? The datasheet is just some not-so-formal syntax to describe something we need to compile in our brains to C source code each time we like to do something with peripheral registers. On 8-bit machines, everything can be easily calculated manually. But by Murphy's law, you will make an error. The manufacturer as a single point of truth (and errata) should supply every developer with usable DFPs, which already contain tested and verified definitions to use. In fact, this is the formal version of the datasheet. 32-bit world now, what does this mean? Do I need to write comments (flavor A or B from above) for every register now? The SERCOM module has more than 10 registers, EVSYS summary table is several pages long.. SERCOM0->USART.CTRLA.reg = 0x40310006; Wait, I've already used the DFPs to access the register: // accessing SERCOM, base address 0x42000400, offset for CTRLA is 0x00 (* (uint32_t*) (0x42000400 + 0x00)) = 0x40310006; 😉 I don't see the point why I as a customer should bother this these numbers for the sake of clarity. In fact, the left side of the statement already is a symbol to access and nobody complains or starts to comment about register and offset calculations, so why would we for the right side?
  4. Best example for unreadable code, you have to have the datasheet open and do some calculations, which might be the first source of bugs. To avoid that, I often find engineers putting comments on top of that line to write down their calculation: // comparator configuration // bit 6 = C1OUT, bit 4 = C1INV => 0101 // bit 3 = CIS, switch to RA0/AN0 // CM<2:0> = 101, one independent comparator CMCON0 = 0x5d; However, this is there the trouble really begins, because the comment can now easily desynchronize with the actual code, because someone had to fix something in a hurry: // comparator configuration // bit 6 = C1OUT, bit 4 = C1INV => 0101 // bit 3 = CIS, switch to RA0/AN0 // CM<2:0> = 101, one independent comparator CMCON0 = 0x55; Where is the bug now? Does the CIS bit have to be switched on according to the comments? Why did the author disable it? Should the comment be updated or left as it is, so that someone with knowledge about it can work on that later? In my opinion, putting the magic number into a #define MAGIC is just a useless layer which simply moves the problem, but may satisfy (company?) guidelines.. So for this particular case of register initialization, I love using Device Family Packs (DFP): SERCOM4_REGS->USART_INT.SERCOM_CTRLA = SERCOM_USART_INT_CTRLA_MODE(1) | SERCOM_USART_INT_CTRLA_RXPO(3) | SERCOM_USART_INT_CTRLA_TXPO(1) | SERCOM_USART_INT_CTRLA_DORD(1); Essentially this is a datasheet in a header file and the manufacturer already did the hard work of defining every bit along with a mask and a name. You still have to know your business if you going to edit this line, but you don't have to fiddle with bare numbers. And the best thing: It all boils down to a constant number before compiling starts.
  5. Welcome to our forum, ingbaker. Can you elaborate your question a bit more, please?
  6. I've got a tip from a colleague: Doxygen also generates dependency and call graphs for a project. So I ran it and after an hour of tweaking and installing things that Doxygen also requires to run I ended up with a nice graph: It doesn't say if the header is actually used by a module, but it certainly is something to get a quick overview of what I'm about to integrate into my project. I always thought of Doxygen being a documentation tool, never thought about the possibility to explore an unknown project with that. Very nice!
  7. I completely agree with you! I try to avoid comments, because they tend to de-synchronize with the actual code very easily. #pragma config PWRTE = ON // Power-up Timer Enable bit->Power up timer disabled What does that mean? Is it a bugfix? Is it a bug? I find this so often in mature projects. Someone edited bit masks for a register, but forgot to update the comments. Even the Hungarian notation as an extended version of commenting de-synchronizes eventually, because you tend to ignore the prefixes after a while. uint32_t u16counter; So my personal approach is to find identifiers and names to formulate a readable code, which says everything what is actually happening on that specific line of code. There is no need to write functions with many lines of code to make the compiling more efficient or save stack space, modern compilers will optimize that out again. Encapsulating a partial solution not only brings structure to your code, it makes the function actually readable. And with readable I mean to read it out loud in front of an audience (e.g. for a code review). Comments should give the reader a general overview of the problem and an abstract strategy what has to be done in a function. If the purpose is obvious and easily derivable from the identifiers, why create a second meta layer which needs extra maintenance and creates a dependency? This in turn often means that an over-commented code doesn't have a good structure or the author doesn't understand the problem well enough to abstract it. Although sometimes the project dictates code metrics, like a code to comment ratio that has to be satisfied. I see a lot of projects with Doxygen comments in it, but the actual content of that documentation is rather unhelpful.
  8. I struggle to import a certain feature into my microcontroller project. One of my "init" projects needs an eeprom emulation feature and I found a fitting solution. However, the project comes with a dozen ASF files which might be needed, maybe not. To further complicate things, I don't need their hardware initialization. So I could strip their system init function, which leads to more potentially dead files in the project. Is there a more convenient way to get an overview of all the dependencies than clicking through all the includes and declarations manually? I have the feeling I don't need most of the files, but the project is a bit too large to rewrite everything. If I would be very lazy, I'd just throw the whole directory in my project. But I guess this won't end well... So what is your opinion?
  9. I was able reproduce that, XC16 and XC32 seem to strip the quotes from your string literal definition, no matter what you do 😞
  10. For me, the trick to strictly read it from right to left helped a lot. volatile List_t * ListHead; A variable ListHead which is a pointer to the type List_t which is volatile. List_t * volatile ListHead; A variable ListHead which is a volatile pointer to a type List_t volatile List_t * volatile ListHead; A variable ListHead which is a volatile pointer to a type List_t which is volatile const int *ptr_to_constant; A variable ptr_to_constant which is a pointer to a type int which is a constant int *const constant_ptr; A variable constant_prt which is a constant pointer to a type int
  11. Not every pin is capable of driving such high frequencies. I strongly suggest to have a look at the output signal with a scope. Are the edges of the signal okay or do they get rounder and rounder with increasing baud rate? Is the frequency correct? A few pins may have special hardware (such as SPI) to have more drive strength, have a Schmitt trigger behavior for inputs etc.
  12. My SAME54 project is growing every day, and I have to make a decision how to manage my code. Maybe someone has an advise for me? The situation is this, I started to develop the project in Studio. Later, some people persuaded my to rebuild my project to work with XC32 along with the headers from XC. So I made a branch "MPLABX_Branch" to put the modified project there. Now I have a complete parallel branch with different register names and it's quite a pain to maintain them both if I want to add something new. Should I put the branch into a new repository? Or should clutter up the code with a lot of #ifdefs for XC and GCC? What would be the best approach?
  13. According to your zip package, PBCLK is set to 48 MHz, not 96MHz? And it's a PIC32MX470F512L, just for clarity 🙂 It would be a good idea to debug your project and have a look at the actual register settings, to see if Harmony did everything right. I would also suggest to check the analog output signal with a scope to check the frequency and the signal quality. Is there a signal at higher baud rates and how does it look like?
  14. And now it works with 2.15. Try to add "extern" to your inline function, this worked for me:
×
×
  • Create New...