Jump to content
 

holdmybeer

Member
  • Content Count

    33
  • Joined

  • Last visited

  • Days Won

    6

holdmybeer last won the day on September 4 2019

holdmybeer had the most liked content!

Community Reputation

9 Neutral

About holdmybeer

  • Rank
    Pupil

Recent Profile Visitors

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

  1. Great idea! Add a magnetic RFID receptacle, then you can set different scenarios by throwing a token against the outside of the trailer. Like Ambient, Astro, Work, Essential only... 😎
  2. Setting ANSEL just disables the digital input. So with ANSEL set to 1, you'll get back 0 regardless which voltage is applied. If you want to make analog measurements, it's a good idea to disable the digital input. Otherwise it'll begin to toggle between 0 and 1 all the time if the voltage is in the forbidden zone (between low and high level). Back when PICs didn't have a dedicated LAT (latch) register, leaving the ANSEL at 1 had a serious side effect: This is from an old PIC16F887. If ANSEL was left at 1, PORT would be always zero. You could still set the output stage to 1 or 0, but you would always read back 0. This leads to a RMW (read-modify-write) problem. To change a particular pin, you had to read out the PORT register for an entire bank, modify the bit for your pin and write it back. If a pin has ANSEL set, it'll return 0, although the output is actually 1. With the write back operation, this pin will be set to 0. So you want to set RA3 to 1, but RA5 (with ANSEL set) gets cleared mysteriously at the same time.. This lead to day-filling debugging sessions and lot of grey hair, so many people still yell "Set ANSEL to zero!!" if you use it as a digital output pin. For modern PICs, this isn't a problem anymore: These devices have a latch register, where you set the logic state for the output buffer. The digital read back is separated, its still available in PORT. If you leave ANSEL set to 1, it'll only affect the PORT register now, LAT is independent from the actual voltage at the pin. Long story short: For modern PICs, you can leave ANSEL set to 1 for digital outputs.
  3. Nice project! How about adding some non-Victron nodes like environmental sensors to the system? Good to see you've thrown out that old lead battery, LiFe batteries are so much better! I have mine for 2 years now and it still has full (60 Ah) capacity. Discharged it twice down to 9.8 V, nothing happened.
  4. 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.
  5. 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))
  6. 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?
  7. 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.
  8. Welcome to our forum, ingbaker. Can you elaborate your question a bit more, please?
  9. 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!
  10. 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.
  11. 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?
  12. I was able reproduce that, XC16 and XC32 seem to strip the quotes from your string literal definition, no matter what you do 😞
  13. 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
  14. 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.
  15. 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?
×
×
  • Create New...