Topics

v6 uBITX TX LPF


Mark Erbaugh
 

There are 4 different TX filters, controlled by 3 signal pins. In the firmware source code, there is are comments:

21 MHz and up: the default filter is with 35 MHz cut-off, (all of the TX_LPF_x pins are low)
14 -21 MHz: thrown the KT1 relay on, the 30 MHz LPF is bypassed and the 14-18 MHz LPF is allowd to go through pin (TX_LPF_A is high, the rest low)

Looking at the v6 schematic, I don't see a KT1 relay. It looks like TX_LPF_A triggers KA1. Also, as I trace the schematic, it appears that one filter (the bottom one on the schematic, I assume the 35 Mhz LPF) is always in the circuit. As I missing something?

73,
Mark, N8ME


Evan Hand
 

Mark,
I am not sure which version of the code you are looking at.  The past practice has been to maintain backward capability to the older boards by selecting features and functions that are added or changed with #define variables. 

There was a significant board redesign around the transmit LPF due to the harmonic issues that were found to be caused by the wiring of that section.   The board change required the relays to be wired differently.  The older versions switched in the filter that was required for the band.  The v5 and v6 leave the 10meter filter in the path and adds the correct lower frequency filter based on the band.

I have not seen a KT1 since I have been on this forum.  I suspect it is a leftover from the single-band BITX20 and BITX40 boards or a typo that has not caused an issue so far.

BTW, there is not a K2 on the board either as that was used in the BITX40 and not in the uBITX versions.  For that change, the relay that does a similar function was renamed to K3.

Above is my understanding of the information that I have reviewed.  I may be wrong or have bad data.  Feedback is welcome.
73
Evan
AC9TU


Mark Erbaugh
 

Evan,

Thanks for the reply. I am referring to the latest v 6.1 firmware downloaded from GitHub. The following snippets are from  ubitx_v6.1_code starting around line 287:

 
/**
 * Select the properly tx harmonic filters
 * The four harmonic filters use only three relays
 * the four LPFs cover 30-21 Mhz, 18 - 14 Mhz, 7-10 MHz and 3.5 to 5 Mhz
 * Briefly, it works like this, 
 * - When KT1 is OFF, the 'off' position routes the PA output through the 30 MHz LPF
 * - When KT1 is ON, it routes the PA output to KT2. Which is why you will see that
 *   the KT1 is on for the three other cases.
 * - When the KT1 is ON and KT2 is off, the off position of KT2 routes the PA output
 *   to 18 MHz LPF (That also works for 14 Mhz) 
 * - When KT1 is On, KT2 is On, it routes the PA output to KT3
 * - KT3, when switched on selects the 7-10 Mhz filter
 * - KT3 when switched off selects the 3.5-5 Mhz filter
 * See the circuit to understand this
 */
 
void setTXFilters(unsigned long freq){
  
  if (freq > 21000000L){  // the default filter is with 35 MHz cut-off
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 0);
  }
  else if (freq >= 14000000L){ //thrown the KT1 relay on, the 30 MHz LPF is bypassed and the 14-18 MHz LPF is allowd to go through
    digitalWrite(TX_LPF_A, 1);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 0);
  }
  else if (freq > 7000000L){
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 1);
    digitalWrite(TX_LPF_C, 0);    
  }
  else {
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 1);    
  }
}
 
 
void setTXFilters_v5(unsigned long freq){
  
  if (freq > 21000000L){  // the default filter is with 35 MHz cut-off
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 0);
  }
  else if (freq >= 14000000L){ //thrown the KT1 relay on, the 30 MHz LPF is bypassed and the 14-18 MHz LPF is allowd to go through
    digitalWrite(TX_LPF_A, 1);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 0);
  }
  else if (freq > 7000000L){
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 1);
    digitalWrite(TX_LPF_C, 0);    
  }
  else {
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 1);    
  }
}
 
It looks like the two functions above are identical (except for their name). I suspect the comments are out of date.

You confirmed my analysis when you said that the 10m filter is always in-line. That's how I interpreted the schematic. I just want to make sure that I understand what's going on with the Arduino control of the hardware as I am working on my own version of the firmware.

73,
Mark, N8ME


Jack, W8TEE
 

All:

First, I'm really happy to see code that uses type qualifiers (e.g., the 'L' at the end of the numbers). They are really not required, but they do help document the code. As a bit of nit picking, the code should be for unsigned long's:

void setTXFilters(unsigned long freq){
  
  if (freq > 21000000UL){  // the default filter is with 35 MHz cut-off

Most programmers just use the magic number in the code with no modifiers. I'd like it even better if is were:

    #define LOWER15MBANDEDGE    21000000UL
// ...bunch of code...

  if (freq > LOWER15MBANDEDGE) {  // the default filter is with 35 MHz cut-off

which would make the code easier to change for foreign hams, if necessary.


Jack, W8TEE

On Wednesday, November 25, 2020, 12:10:55 PM EST, Mark Erbaugh <mark.election@...> wrote:


Evan,

Thanks for the reply. I am referring to the latest v 6.1 firmware downloaded from GitHub. The following snippets are from  ubitx_v6.1_code starting around line 287:

 
/**
 * Select the properly tx harmonic filters
 * The four harmonic filters use only three relays
 * the four LPFs cover 30-21 Mhz, 18 - 14 Mhz, 7-10 MHz and 3.5 to 5 Mhz
 * Briefly, it works like this, 
 * - When KT1 is OFF, the 'off' position routes the PA output through the 30 MHz LPF
 * - When KT1 is ON, it routes the PA output to KT2. Which is why you will see that
 *   the KT1 is on for the three other cases.
 * - When the KT1 is ON and KT2 is off, the off position of KT2 routes the PA output
 *   to 18 MHz LPF (That also works for 14 Mhz) 
 * - When KT1 is On, KT2 is On, it routes the PA output to KT3
 * - KT3, when switched on selects the 7-10 Mhz filter
 * - KT3 when switched off selects the 3.5-5 Mhz filter
 * See the circuit to understand this
 */
 
void setTXFilters(unsigned long freq){
  
  if (freq > 21000000L){  // the default filter is with 35 MHz cut-off
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 0);
  }
  else if (freq >= 14000000L){ //thrown the KT1 relay on, the 30 MHz LPF is bypassed and the 14-18 MHz LPF is allowd to go through
    digitalWrite(TX_LPF_A, 1);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 0);
  }
  else if (freq > 7000000L){
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 1);
    digitalWrite(TX_LPF_C, 0);    
  }
  else {
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 1);    
  }
}
 
 
void setTXFilters_v5(unsigned long freq){
  
  if (freq > 21000000L){  // the default filter is with 35 MHz cut-off
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 0);
  }
  else if (freq >= 14000000L){ //thrown the KT1 relay on, the 30 MHz LPF is bypassed and the 14-18 MHz LPF is allowd to go through
    digitalWrite(TX_LPF_A, 1);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 0);
  }
  else if (freq > 7000000L){
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 1);
    digitalWrite(TX_LPF_C, 0);    
  }
  else {
    digitalWrite(TX_LPF_A, 0);
    digitalWrite(TX_LPF_B, 0);
    digitalWrite(TX_LPF_C, 1);    
  }
}
 
It looks like the two functions above are identical (except for their name). I suspect the comments are out of date.

You confirmed my analysis when you said that the 10m filter is always in-line. That's how I interpreted the schematic. I just want to make sure that I understand what's going on with the Arduino control of the hardware as I am working on my own version of the firmware.

73,
Mark, N8ME

--
Jack, W8TEE


Jerry Gaffke
 

Jack,

Good thing you can write your own code.  ;-)


I consider those to be some rather small nits.
Here's my own take, pushing back a bit to make folks feel better about
finding a coding style they are comfortable with.

I find 
  if (freq > 21000000)  {                  // The lower 15meter band edge in Hz
much easier to read than 

  #define LOWER15MBANDEDGE    21000000UL
  // ...  hundreds of lines of code ...
  if (freq > LOWER15MBANDEDGE) {

A #define makes sense if the value is used in more than one place,
but that is not the case in the above example.

I can read LOWER15MBANDEDGE almost as a comment.
But if we are targeting foreign hams that may not be fluent in English,
it's better to write a proper comment that fully describes what is going on.
Something they can send through google translate.

It often does make good sense to put stuff like
  #define LOWER15MBANDEDGE    21000000
up at the start of the file with copious comments so somebody
can make necessary changes without bothering to read any C code.
That is what I would tend to do if coding for other than myself.
But unless the time is taken to write lots of good comments around those defines, I'd need
to follow the code to see what each one is doing before I'd be comfortable hacking the value.

If you can't bring yourself to fully document it, consider just having a comment at the top stating
"To change band edges, modify the values inside setTXFilters()"

Also, I know perfectly well what the compiler will do with
    if (freq > 21000000){
Adding unnecessary U's and L's just makes it harder to count the digits.

Take a hard look at the si5351bx routines in the uBitx code base.
There are a very few critical #defines at the top with what I thought were good comments,
lots of magic numbers down in the code. 
Rewrite it so it uses #define's for all constants, properly identifies
all unsigned constants with a U, marks anything longer than 16 bits with an L.
If you think the result is easier to read than the original, I beg to differ. 
Strenuously.

To each their own.

Jerry,   KE7ER


On Wed, Nov 25, 2020 at 09:22 AM, Jack, W8TEE wrote:
First, I'm really happy to see code that uses type qualifiers (e.g., the 'L' at the end of the numbers). They are really not required, but they do help document the code. As a bit of nit picking, the code should be for unsigned long's:
 
void setTXFilters(unsigned long freq){
  
  if (freq > 21000000UL){  // the default filter is with 35 MHz cut-off
 
Most programmers just use the magic number in the code with no modifiers. I'd like it even better if is were:


Mark Erbaugh
 

I use a lot of constants in the code that I write. I prefer constants to hard coded numbers, and I strive to have the constants have meaning. In th\is case, I don't think the constant LOWER15MBANDEDGE is appropriate. The value (21 MHz) is not dependent on the operating band, but rather on the physical characteristic of the TX filter. That doesn't change if the band allocation changes.

Secondly, I stay away from #define's for constants. I prefer to use a const, i.e. const unsigned long FILTER3_BOTTOM = 21000000ul;

73,
Mark, N8ME


AndyH
 

Hey Mark,

   K1's the transmit/receive relay - activated by the transmit signal.  I suspect that's how 'KT' was originally labeled.  It's in the lower left of the V6 schematic.

   73, Andy


AndyH
 

Looking at earlier µBITX schematics, the lowpass filters on the V3 µBITX were switched by 'KT1' through 'KT3'.  Maybe the code reference is vestigial.

73


On Wed, Nov 25, 2020 at 02:02 PM, AndyH wrote:
Hey Mark,

   K1's the transmit/receive relay - activated by the transmit signal.  I suspect that's how 'KT' was originally labeled.  It's in the lower left of the V6 schematic.

   73, Andy


Jack, W8TEE
 

Jerry:

First, magic numbers in code are often not as recognizable at those presented here. Second, if you had read the code closely, you would have seen that those constants are used in two different functions: setTXFilters() and setTXFilters_V5(). Third, any time you touch the source code, you create the possibility of messing things up. Symbolic constants reduce the need to touch the code more than once. Fourth, a good symbolic name helps document what the constant is, especially when its value is uncommon. True, most people know what Pi is, yet virtually every compiler I know of has a symbolic constant for Pi. I wonder why? Fifth, I break out all but the most trivial programs into multiple files so there is only one INO file, but often many CPP (i.e., C Plus Plus) files, and a H header file. This allows me to take advantage of incremental compiles, which can save a huge amount of time, and it allows me to collect things like symbolic constants into once place. If I write a header file that contains:

#define LOWER80MBANDEDGE     3500000UL        // Lower band edge in US
#define LOWER40MBANDEDGE     7000000UL
#define LOWER20MBANDEDGE    14000000UL
#define LOWER15MBANDEDGE    21000000UL
#define LOWER10MBANDEDGE    28000000UL

and I'm from a country that doesn't share the same band edges, I can go to this one place in the code and make the changes and know that they are changed everywhere necessary in the code. Your approach can't do that and needs to find every occurrence of the magic number and then make the change. Also if you use symbolic constants, the reader doesn't see the number of the "UL", only an symbolic entry that makes sense by itself.

As to the si5351 code file in the µBITX code base, it does contain 7 symbolic constants (#define's) already and would actually benefit from a few more. In fact, some of those are macro expansions which is also good since #define's are "typeless". That is, this macro:

    #define ARRAYELEMENTS(x)   (sizeof(x) / sizeof(x[0]))

allows me to get rid of ANY magic number the refers to the size of the array regardless of data type of the array. So instead of magic numbers:

    int myArray[10];

    // a bunch of code...

    for (i = 0; i < 10; i++) {
        // whatever...
    }

I can get rid of the magic number 10 and use:

    for (i = 0; i < ARRAYELEMENTS(myArray); i++) {
        // whatever...
    }

If I later change the size of myArray[], even if I use a similar loop a hundred times in the code, It doesn't matter if the array is a char, int, long, float, or even a struct, because its typeless, it will work. I only need to make 1 change in the source code and all uses throughout the code are changed correctly. Your method would be more error prone.

But then, that's the beauty of knowing how to program. You can make it as easy or hard to do as you wish.

Jack, W8TEE




On Wednesday, November 25, 2020, 1:52:34 PM EST, Jerry Gaffke via groups.io <jgaffke@...> wrote:


Jack,

Good thing you can write your own code.  ;-)


I consider those to be some rather small nits.
Here's my own take, pushing back a bit to make folks feel better about
finding a coding style they are comfortable with.

I find 
  if (freq > 21000000)  {                  // The lower 15meter band edge in Hz
much easier to read than 

  #define LOWER15MBANDEDGE    21000000UL
  // ...  hundreds of lines of code ...
  if (freq > LOWER15MBANDEDGE) {

A #define makes sense if the value is used in more than one place,
but that is not the case in the above example.

I can read LOWER15MBANDEDGE almost as a comment.
But if we are targeting foreign hams that may not be fluent in English,
it's better to write a proper comment that fully describes what is going on.
Something they can send through google translate.

It often does make good sense to put stuff like
  #define LOWER15MBANDEDGE    21000000
up at the start of the file with copious comments so somebody
can make necessary changes without bothering to read any C code.
That is what I would tend to do if coding for other than myself.
But unless the time is taken to write lots of good comments around those defines, I'd need
to follow the code to see what each one is doing before I'd be comfortable hacking the value.

If you can't bring yourself to fully document it, consider just having a comment at the top stating
"To change band edges, modify the values inside setTXFilters()"

Also, I know perfectly well what the compiler will do with
    if (freq > 21000000){
Adding unnecessary U's and L's just makes it harder to count the digits.

Take a hard look at the si5351bx routines in the uBitx code base.
There are a very few critical #defines at the top with what I thought were good comments,
lots of magic numbers down in the code. 
Rewrite it so it uses #define's for all constants, properly identifies
all unsigned constants with a U, marks anything longer than 16 bits with an L.
If you think the result is easier to read than the original, I beg to differ. 
Strenuously.

To each their own.

Jerry,   KE7ER

On Wed, Nov 25, 2020 at 09:22 AM, Jack, W8TEE wrote:
First, I'm really happy to see code that uses type qualifiers (e.g., the 'L' at the end of the numbers). They are really not required, but they do help document the code. As a bit of nit picking, the code should be for unsigned long's:
 
void setTXFilters(unsigned long freq){
  
  if (freq > 21000000UL){  // the default filter is with 35 MHz cut-off
 
Most programmers just use the magic number in the code with no modifiers. I'd like it even better if is were:

--
Jack, W8TEE


Jack, W8TEE
 

Mark:

The problem with using const is that the data type is "fixed" whereas a #define is typeless. See my post to Jerry. For example, if I used your example:

    const unsigned long FILTER3_BOTTOM = 21000000ul;

and I decided it needs to be a signed long, you have to go and change it to:

    const long FILTER3_BOTTOM = 21000000l;

Had you used a symbolic constant, you just drop the 'u'. Also, I would never use lower case letters for data type specifiers, especially for a long data type, such as:

    const long FILTEROFFSET = 250l;

because the 'L' at the end looks too much like a one's digit character '1'.

Jack, W8TEE


On Wednesday, November 25, 2020, 2:36:40 PM EST, Mark Erbaugh <mark.election@...> wrote:


I use a lot of constants in the code that I write. I prefer constants to hard coded numbers, and I strive to have the constants have meaning. In th\is case, I don't think the constant LOWER15MBANDEDGE is appropriate. The value (21 MHz) is not dependent on the operating band, but rather on the physical characteristic of the TX filter. That doesn't change if the band allocation changes.

Secondly, I stay away from #define's for constants. I prefer to use a const, i.e. const unsigned long FILTER3_BOTTOM = 21000000ul;

73,
Mark, N8ME

--
Jack, W8TEE


Evan Hand
 

On Wed, Nov 25, 2020 at 11:10 AM, Mark Erbaugh wrote:
You confirmed my analysis when you said that the 10m filter is always in-line. That's how I interpreted the schematic. I just want to make sure that I understand what's going on with the Arduino control of the hardware as I am working on my own version of the firmware.
Mark,
There is another source of code for the v6 written by Reed N.  You might want to review his code to see the improvements that he made and maybe get some other ideas.  His code is here:
https://github.com/reedbn/ubitxv6

I can see why the source code documentation can be confusing.  It would be nice if Ashhar Farhan would clean up the documents, however, it does work and the kit is the best performance for the price of an SSB rig, so far (there is at least one the Xiego G90 that is getting close), and it is the best open-source selection that I am aware of.  The only disadvantage is that the hardware is not open-source in that the board layout files are not available.  The schematic of course is.

Good luck and have fun with the mods!
73
Evan
AC9TU


Jerry Gaffke
 

Jack,

The specifics of any particular example determine how best to do it.
For me at least, the primary rule is "don't unnecessarily obfuscate".
And I've seen lots of code that is unnecessarily obfuscated through unneeded indirection.

I am curious what the si5351bx routines would look like had you done them.
Seems using  #define's and/or a const's  on all those magic numbers would
just get in the way, making it considerably harder to figure out what the code does.  
Note that where someone might want to change them, I have brought
them out up front as #defines, with comments telling what they do.

As I say, to each their own.
Those writing code for the uBitx should feel free to find what works best for them
without worrying about somebody else's notion of correct coding style.

My next rant will be about the people who have told me
that my C code should be "object oriented".   ;-)

Jerry, KE7ER


Jack, W8TEE
 

Jerry:

I agree. Each person should write what they feel works best for them. I think symbolic constants make the code more clear as long as the constant names are clear. You feel a pure number is more clear.

As to the si5351 code, I think the following code:

    msa = si5351bx_vcoa / fout;     // Integer part of vco/fout
    msb = si5351bx_vcoa % fout;     // Fractional part of vco/fout
    msc = fout;             // Divide by 2 till fits in reg
    while (msc & 0xfff00000) {
      msb = msb >> 1;
      msc = msc >> 1;
    }
    msxp1 = (128 * msa + 128 * msb / msc - 512) | (((uint32_t)si5351bx_rdiv) << 20);
    msxp2 = 128 * msb - 128 * msb / msc * msc; // msxp3 == msc;
    msxp3p2top = (((msc & 0x0F0000) << 4) | msxp2);     // 2 top nibbles
    uint8_t vals[8] = { BB1(msc), BB0(msc), BB2(msxp1), BB1(msxp1),
                        BB0(msxp1), BB2(msxp3p2top), BB1(msxp2), BB0(msxp2)
                      };
    i2cWriten(42 + (clknum * 8), vals, 8); // Write to 8 msynth regs

could be written in a way that makes it easier to understand. As it is, it kinda makes me wonder what 128, 512, and 42 are in the code and a lot of the variables would benefit from more descriptive names. True, if I study the code, I can figure out what they are, but if I used symbolic constants for them, I guarantee it would be easier for most people to read with less time spent studying the code.

I also don't appreciate code that tries to "show off". For example, what does this do:

  int matrix[5][5];
  int i, j;

  Serial.begin(9600);
 
  for (i = 0; i < 5; i++) {
    for (j = 0; j < 5; j++) {
      matrix[i][j] = (j/i) * (i/j);
    }
  }

and can you think of a less "obtuse" way of doing the same thing? When I was teaching, I always favored code that was clearly written over clever code that might make 1 person out of a 1000 say "That's pretty cool!

But, again, there are no rules etched in stone.

Jack, W8TEE


On Wednesday, November 25, 2020, 4:08:15 PM EST, Jerry Gaffke via groups.io <jgaffke@...> wrote:


Jack,

The specifics of any particular example determine how best to do it.
For me at least, the primary rule is "don't unnecessarily obfuscate".
And I've seen lots of code that is unnecessarily obfuscated through unneeded indirection.

I am curious what the si5351bx routines would look like had you done them.
Seems using  #define's and/or a const's  on all those magic numbers would
just get in the way, making it considerably harder to figure out what the code does.  
Note that where someone might want to change them, I have brought
them out up front as #defines, with comments telling what they do.

As I say, to each their own.
Those writing code for the uBitx should feel free to find what works best for them
without worrying about somebody else's notion of correct coding style.

My next rant will be about the people who have told me
that my C code should be "object oriented".   ;-)

Jerry, KE7ER

--
Jack, W8TEE


Mark Erbaugh
 

Jack,

What is the issue with the const having a fixed data type? In your example, wouldn't the compiler just coerce the unsigned long to a long with no runtime penalty?

Is there a difference between 'UL' and 'ul'? If not, I would agree that the upper-case versions would be less ambiguous.

73,
Mark


Bob Lunsford
 

I do not know why it was an issue but decades ago, maybe before "C" ... I read one programming guru say that what should be avoided is overuse of the GOTO subroutines. Said it can slow systems down. Now, that was before our modern-day computers where processing speed is not an issue was when I read that. Hmmm, could have been directed to programmers in Basic? It was that long ago, anyway.

Bob — KK5R

On Wednesday, November 25, 2020, 4:08:16 PM EST, Jerry Gaffke via groups.io <jgaffke@...> wrote:


Jack,

The specifics of any particular example determine how best to do it.
For me at least, the primary rule is "don't unnecessarily obfuscate".
And I've seen lots of code that is unnecessarily obfuscated through unneeded indirection.

I am curious what the si5351bx routines would look like had you done them.
Seems using  #define's and/or a const's  on all those magic numbers would
just get in the way, making it considerably harder to figure out what the code does.  
Note that where someone might want to change them, I have brought
them out up front as #defines, with comments telling what they do.

As I say, to each their own.
Those writing code for the uBitx should feel free to find what works best for them
without worrying about somebody else's notion of correct coding style.

My next rant will be about the people who have told me
that my C code should be "object oriented".   ;-)

Jerry, KE7ER


Jack, W8TEE
 

Mark:

Back in the 80's, my company produced a C compiler for MSDOS. We had a "picky flag", sort of like Lint, that checked for syntax/semantic errors. I have never seen any published code that would pass our Picky 10. Indeed, it was so fussy, we developed at Picky 4 and only bumped up to Picky 10 when we "knew" there were no errors. (There were still a few.)

One of those errors was what I call a "silent cast". A silent cast occurs when the two data types on either side of the assignment operator don't match. I've seen compilers in the past that tried to jam a 4-byte long on the right side of the expression into a single byte on the left side of the expression. Despite slopping 3 bytes of data all over the floor, not a peep from the parser...a silent cast of the worst type. I'm not sure about all compilers, but my guess is that most do not inflict a penalty for an unsigned long to long cast. Most compilers are way smarter than we are, so they will likely get it right in all cases and likely without error.

However, perhaps the better question is why are you flip-flopping between data types? If you mean to use an unsigned long, then use it. My reason for doing so is because the si5351 library functions ask for an unsigned long. While we Arduino IDE programmers live in a world of Open Source and can inspect the library source code, that's not the case on all platforms everywhere. Function libraries and methods should be the equivalent of Las Vegas...what goes on inside that black box is none of our business. Given that is true for many development platforms, as a conscientious programmer I have to assume they asked for an unsigned long and not a long for a reason, so I'll play by their rules. I have no idea how many bites in the butt this has saved me, but my guess there are some.

I write for clarity...not for tomorrow, but for six months or years from now when I need to understand what the code does. I even stress this in my beginning C books. Optimizing compilers probably do a better job of tweaking the generated code than I can, so I leave code generation to them. I write so that I, and anyone else on my programming team, can understand what I did. If one of my colleagues sees 9600L in the code, they know that the affected variable is defined as a long. Likewise, they trust me enough to know that if I write 9600UL, they know they can bet their first born that the affected variable is an unsigned long. Again, it's a form of documentation that lessens equivocation.

There is no difference to the compiler between lowercase and uppercase type specifiers, so I always use uppercase. It likely causes less confusion with the digit character '1'.

Jack, W8TEE

On Wednesday, November 25, 2020, 5:32:52 PM EST, Mark Erbaugh <mark.election@...> wrote:


Jack,

What is the issue with the const having a fixed data type? In your example, wouldn't the compiler just coerce the unsigned long to a long with no runtime penalty?

Is there a difference between 'UL' and 'ul'? If not, I would agree that the upper-case versions would be less ambiguous.

73,
Mark

--
Jack, W8TEE


Jerry Gaffke
 

Jack,

I do tend to scrunch my code up so I can study it all in one screen.
And given that this was a habit learned on an 80x24 ADM-3 display,
my code tends to be scrunched indeed.

One thing that some might find overly obtuse is:

uint8_t vals[8] = { BB1(msc), BB0(msc), BB2(msxp1), BB1(msxp1),
                        BB0(msxp1), BB2(msxp3p2top), BB1(msxp2), BB0(msxp2)

Initializing an array like that in the middle of a bunch of code is somewhat odd, but far more
concise than 
creating the array and making 8 assignment statements.  I think it's a win.

Otherwise, the reason the code is so murky is because the Si5351 is murky.
The code could use more comments, but sufficient comments to make the Si5351 understandable
would make it roughly the size of the SiLabs AN619 app note.
Loading it up with #defines for all the magic numbers would not make it more readable.

Regarding your puzzle, since i and j start at zero, it's a dandy way to get a core dump on divide-by-zero.
If i and j start at 1, you get a nice identity matrix with a diagonal of 1's, all other entries are zero.
And if the (i/j)*(j/i) is done in floating point with i and j starting at 1, the whole matrix is 1's.
I assume it is the identity matrix that you are going for, and while it is sort of cool, this is
just as concise and less obfuscated, works with indices of zero:   matrix[i][j] = i==j?1:0;
Or if you wish, even less obfuscated:    if (i==j) matrix[i][j]=1; else matrix[i][j]=0;

Regarding this business of tacking U and/or L on the end of integer constants,
the first answer here seems quite correct:
    https://stackoverflow.com/questions/13134956/what-is-the-reason-for-explicitly-declaring-l-or-ul-for-long-values

Specifically: 

A common reason to use a suffix is ensuring that the result of a computation doesn't overflow. Two examples are:
long x = 10000L * 4096L;
unsigned long long y = 1ULL << 36;

That is about the only case where I find reason to bother with informing the compiler how big my constant should be.
As that stackoverflow answer tells us, the compiler is perfectly capable of figuring out that it needs a 32 bit word,
and whether it might be negative.  
I find it easier to read without the extra UL on th end.

Perhaps C compilers of 1980 weren't quite so smart. 
I don't really remember, and I've lost my first edition of K&R.  
That could well explain why some insist on all the U's and L's.

Jerry, KE7ER


On Wed, Nov 25, 2020 at 02:08 PM, Jack, W8TEE wrote:
Jerry:
 
I agree. Each person should write what they feel works best for them. I think symbolic constants make the code more clear as long as the constant names are clear. You feel a pure number is more clear.
 
As to the si5351 code, I think the following code:
 
    msa = si5351bx_vcoa / fout;     // Integer part of vco/fout
    msb = si5351bx_vcoa % fout;     // Fractional part of vco/fout
    msc = fout;             // Divide by 2 till fits in reg
    while (msc & 0xfff00000) {
      msb = msb >> 1;
      msc = msc >> 1;
    }
    msxp1 = (128 * msa + 128 * msb / msc - 512) | (((uint32_t)si5351bx_rdiv) << 20);
    msxp2 = 128 * msb - 128 * msb / msc * msc; // msxp3 == msc;
    msxp3p2top = (((msc & 0x0F0000) << 4) | msxp2);     // 2 top nibbles
    uint8_t vals[8] = { BB1(msc), BB0(msc), BB2(msxp1), BB1(msxp1),
                        BB0(msxp1), BB2(msxp3p2top), BB1(msxp2), BB0(msxp2)
                      };
    i2cWriten(42 + (clknum * 8), vals, 8); // Write to 8 msynth regs

could be written in a way that makes it easier to understand. As it is, it kinda makes me wonder what 128, 512, and 42 are in the code and a lot of the variables would benefit from more descriptive names. True, if I study the code, I can figure out what they are, but if I used symbolic constants for them, I guarantee it would be easier for most people to read with less time spent studying the code.
 
I also don't appreciate code that tries to "show off". For example, what does this do:
 
  int matrix[5][5];
  int i, j;

  Serial.begin(9600);
 
  for (i = 0; i < 5; i++) {
    for (j = 0; j < 5; j++) {
      matrix[i][j] = (j/i) * (i/j);
    }
  }
 
and can you think of a less "obtuse" way of doing the same thing? When I was teaching, I always favored code that was clearly written over clever code that might make 1 person out of a 1000 say "That's pretty cool!
 
But, again, there are no rules etched in stone.
 
Jack, W8TEE
 


Jerry Gaffke
 

Jack,

I don't think you have yet explained why
    #define LOWER15MBANDEDGE    21000000UL
should have that "UL" at the end.
This works fine and is easier to read:
    #define LOWER15MBANDEDGE    21000000

There are lots of things in C that make low level programming easy.
Stuffing a 32 bit word into an 8 bit register is one of them.
Then downshifting and stuffing it into a second register.
This is quite readable if you understand the environment, and very concise.
I'd be pissed if the compiler started insisting on masking off the unused bits and doing a typecast.

Jerry, KE7ER


Jack, W8TEE
 

Jerry:

You would think the matrix code would flag a divide by zero error, but it doesn't. Load up the IDE and give it a try. Also, I would never use that code in anything but a teaching moment because there are much clearer ways to accomplish the same thing. Indeed, your if statement block would be the way I would write it:

 if (i==j)
    matrix[i][j]=1;
else
    matrix[i][j]=0;

and I would spread it out over 4 lines rather than squish it up on one line. I would never use the ternary form:

matrix[i][j] = i==j?1:0;

because the if block is easier to read and understand. All three variations will generate exactly the same code, so I opt for the first version because it's easier to read.

This statement:

     the compiler is perfectly capable of figuring out that it needs a 32 bit word

confirms you don't understand my purpose for using the data type specifiers. You aren't writing code for the compiler, your writing the code so someone can read and easily understand it. If you are writing for the compiler and want the code to fit onto one screen, then you should write your code as:

int matrix[5][5];int i,j;Serial.begin(9600);for(i=0;i<5;i++)for(j=0;j<5j++)matrix[i][j]=(j/i)*(i/j);}}
 
Like it or not, you don't write code like the above because you know you need to read it at some point. It's the same for using U or L. They are to make the code more readable by humans, not a parsing aid to the compiler.

Jack, W8TEE


On Wednesday, November 25, 2020, 10:35:49 PM EST, Jerry Gaffke via groups.io <jgaffke@...> wrote:


Jack,

I do tend to scrunch my code up so I can study it all in one screen.
And given that this was a habit learned on an 80x24 ADM-3 display,
my code tends to be scrunched indeed.

One thing that some might find overly obtuse is:

uint8_t vals[8] = { BB1(msc), BB0(msc), BB2(msxp1), BB1(msxp1),
                        BB0(msxp1), BB2(msxp3p2top), BB1(msxp2), BB0(msxp2)

Initializing an array like that in the middle of a bunch of code is somewhat odd, but far more
concise than 
creating the array and making 8 assignment statements.  I think it's a win.

Otherwise, the reason the code is so murky is because the Si5351 is murky.
The code could use more comments, but sufficient comments to make the Si5351 understandable
would make it roughly the size of the SiLabs AN619 app note.
Loading it up with #defines for all the magic numbers would not make it more readable.

Regarding your puzzle, since i and j start at zero, it's a dandy way to get a core dump on divide-by-zero.
If i and j start at 1, you get a nice identity matrix with a diagonal of 1's, all other entries are zero.
And if the (i/j)*(j/i) is done in floating point with i and j starting at 1, the whole matrix is 1's.
I assume it is the identity matrix that you are going for, and while it is sort of cool, this is
just as concise and less obfuscated, works with indices of zero:   matrix[i][j] = i==j?1:0;
Or if you wish, even less obfuscated:    if (i==j) matrix[i][j]=1; else matrix[i][j]=0;

Regarding this business of tacking U and/or L on the end of integer constants,
the first answer here seems quite correct:
    https://stackoverflow.com/questions/13134956/what-is-the-reason-for-explicitly-declaring-l-or-ul-for-long-values

Specifically: 

A common reason to use a suffix is ensuring that the result of a computation doesn't overflow. Two examples are:
long x = 10000L * 4096L;
unsigned long long y = 1ULL << 36;

That is about the only case where I find reason to bother with informing the compiler how big my constant should be.
As that stackoverflow answer tells us, the compiler is perfectly capable of figuring out that it needs a 32 bit word,
and whether it might be negative.  
I find it easier to read without the extra UL on th end.

Perhaps C compilers of 1980 weren't quite so smart. 
I don't really remember, and I've lost my first edition of K&R.  
That could well explain why some insist on all the U's and L's.

Jerry, KE7ER


On Wed, Nov 25, 2020 at 02:08 PM, Jack, W8TEE wrote:
Jerry:
 
I agree. Each person should write what they feel works best for them. I think symbolic constants make the code more clear as long as the constant names are clear. You feel a pure number is more clear.
 
As to the si5351 code, I think the following code:
 
    msa = si5351bx_vcoa / fout;     // Integer part of vco/fout
    msb = si5351bx_vcoa % fout;     // Fractional part of vco/fout
    msc = fout;             // Divide by 2 till fits in reg
    while (msc & 0xfff00000) {
      msb = msb >> 1;
      msc = msc >> 1;
    }
    msxp1 = (128 * msa + 128 * msb / msc - 512) | (((uint32_t)si5351bx_rdiv) << 20);
    msxp2 = 128 * msb - 128 * msb / msc * msc; // msxp3 == msc;
    msxp3p2top = (((msc & 0x0F0000) << 4) | msxp2);     // 2 top nibbles
    uint8_t vals[8] = { BB1(msc), BB0(msc), BB2(msxp1), BB1(msxp1),
                        BB0(msxp1), BB2(msxp3p2top), BB1(msxp2), BB0(msxp2)
                      };
    i2cWriten(42 + (clknum * 8), vals, 8); // Write to 8 msynth regs

could be written in a way that makes it easier to understand. As it is, it kinda makes me wonder what 128, 512, and 42 are in the code and a lot of the variables would benefit from more descriptive names. True, if I study the code, I can figure out what they are, but if I used symbolic constants for them, I guarantee it would be easier for most people to read with less time spent studying the code.
 
I also don't appreciate code that tries to "show off". For example, what does this do:
 
  int matrix[5][5];
  int i, j;

  Serial.begin(9600);
 
  for (i = 0; i < 5; i++) {
    for (j = 0; j < 5; j++) {
      matrix[i][j] = (j/i) * (i/j);
    }
  }
 
and can you think of a less "obtuse" way of doing the same thing? When I was teaching, I always favored code that was clearly written over clever code that might make 1 person out of a 1000 say "That's pretty cool!
 
But, again, there are no rules etched in stone.
 
Jack, W8TEE
 

--
Jack, W8TEE


Jack, W8TEE
 

I think I did explain it quite clearly in an earlier post. A 32-bit compiler can handle the numerics below as an int or a long, or an unsigned long. Because the si5351 library expects an unsigned long, the UL simply documents that fact. Your constant could be an int, long, or unsigned long on an STM32, ESP32, or Teensy T4. You're right, the compiler will get it right either way.

You prefer to write for the compiler. I prefer to write for the compiler and the reader. It's just a difference in points of view.

Jack, W8TEE

On Wednesday, November 25, 2020, 10:57:23 PM EST, Jerry Gaffke via groups.io <jgaffke@...> wrote:


Jack,

I don't think you have yet explained why
    #define LOWER15MBANDEDGE    21000000UL
should have that "UL" at the end.
This works fine and is easier to read:
    #define LOWER15MBANDEDGE    21000000

There are lots of things in C that make low level programming easy.
Stuffing a 32 bit word into an 8 bit register is one of them.
Then downshifting and stuffing it into a second register.
This is quite readable if you understand the environment, and very concise.
I'd be pissed if the compiler started insisting on masking off the unused bits and doing a typecast.

Jerry, KE7ER

--
Jack, W8TEE