So I've been doing some devving using a PIC16F15345 (though this is by NO means a device-specific issue), and specifically, using the hardware serial port access commands. Anyway, at 9600baud, I should be able to reach 960 bytes per second transmission. I'm sending right around 1000 bytes...but it takes three seconds to send that!
An oscilloscope showed that the comms are NOT back-to-back like they could be. (Note that I am running the PIC at 1MHz, so there may be gaps as the PIC gets the next transmission ready...but not on EVERY character.) However, looking in the compiled ASM output, I found the following:
HSERSEND
;Block before sending (if needed)
;Send byte
;Registers/Bits determined by #samevar at top of file
;if comport = 1 Then
decf COMPORT,W
btfss STATUS, Z
goto ENDIF49
;HSerSendBlocker
;Wait While TXIF = Off
SysWaitLoop3
banksel PIR3
btfss PIR3,TX1IF
goto SysWaitLoop3
;asm showdebug TXREG equals SerData below will assign SerData to TXREG or TXREG1 or U1TXB via the #samevar
;txreg equals serdata below will assign serdata to txreg | txreg1 | txreg via the #samevar
;
;TXREG = SerData
banksel SERDATA
movf SERDATA,W
banksel TXREG
movwf TXREG
;Add USART_DELAY After all bits are shifted out
;Wait until TRMT = 1
SysWaitLoop4
btfss TX1STA,TRMT
goto SysWaitLoop4
;Add USART_DELAY After all bits are shifted out
;Wait USART_DELAY
movlw 1
movwf SysWaitTempMS
clrf SysWaitTempMS_H
banksel STATUS
call Delay_MS
;exit sub
return
;end if
ENDIF49
return
Note that I have defined "USART_TX_BLOCKING", which adds the "HSerSendBlocker, wait while TXIF is off." In my (not very) humble opinion, that should be the default setting.
I am at a complete loss to understand the purpose of any lines of code after "movwf TXREG". All they do is completely waste CPU cycles that could be used to "multitask" while the EUSART is sending the byte. Not only does the code wait for the EUSART to finish sending the byte (polling TRMT), it also adds a delay on top of that! No wonder I can't get anywhere close to the 960 byte theoretical maximum @9600baud.
Found the source in "usart.h", with a comment: "'8/2/2010: Added delay after each byte in print." What was the reason? I understand that GCB supports a LARGE array of MCUs, but this doesn't make any sense.
"9/12/2015: Changed default USART_DELAY from 12 ms back to 1 ms - fix very slow USART default!" 12mS after EVERY character? What???!
Looking further through USART.H, I notice that there are NO delays for the AVR implementation, though the "HSerSendBlocker" is still optional, for some strange reason.
I deleted the offending lines out of USART.H. Transmit time for my 1,000 byte transmission? Just over a second (don't have an official means of measuring it.) Talk about an improvement...! (Saved some program memory, to boot.)
Last edit: Siddy 2019-08-10
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
You have found code that is not documented when Bill, the developer gentlemen that last adapted the library.
Add #define USART_DELAY OFF to your code and this will optmise.
#chip PIC16F15345
#define USART_DELAY OFF
'USART settings
#define USART_BAUD_RATE 9600 'Initializes USART port with 9600 baud
hsersend 6
gives
HSERSEND
;Block before sending (if needed)
;Send byte
;Registers/Bits determined by #samevar at top of file
;if comport = 1 Then
decf COMPORT,W
btfss STATUS, Z
goto ENDIF1
;HSerSendBlocker
;asm showdebug TXREG equals SerData below will assign SerData to TXREG or TXREG1 or U1TXB via the #samevar
;txreg equals serdata below will assign serdata to txreg | txreg1 | txreg via the #samevar
;
;TXREG = SerData
movf SERDATA,W
banksel TXREG
movwf TXREG
;Add USART_DELAY After all bits are shifted out
;Wait until TRMT = 1
SysWaitLoop1
btfss TX1STA,TRMT
goto SysWaitLoop1
;exit sub
banksel STATUS
return
;end if
ENDIF1
return
The btfss TX1STA,TRMT after the movwf TXREG...
The TRMT bit of the TX1STA register indicates the
status of the TSR register. This is a read-only bit. The
TRMT bit is set when the TSR register is empty and is
cleared when a character is transferred to the TSR
register from the TX1REG. The TRMT bit remains clear
until all bits have been shifted out of the TSR register. No interrupt logic is tied to this bit, so the user has to
poll this bit to determine the TSR status.
Regarding the comments... they can always be improved. We are trying to clean up.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Regarding polling of TRMT: Attached to this post are the timing diagrams from the Microchip PIC16F15345 datasheet, DS40001865B, pages 429-430 (which are by NO means specific to this PIC MCU--it's just what I'm currently working with). One is "Async Single", which shows the waveforms for a single transmit. Notice that TRMT is held LOW until the EUSART has finished sending the entire byte. Waiting for TRMT to rise before returning has the exact code equivalent of sending the RS-232 data via bit-banging--COMPLETELY neutralizing the "transmit" benefit of the EUSART module. The reason there is no interrupt logic tied to TRMT, is because it is not needed for general data transmission. The only time you would need TRMT is to determine if the I/O port is presently sending data, say, to assert a flow-control line. Or to make sure that the last byte of guidance data has actually been sent before your model rocket launches off on that top secret moon mission to retrieve Alan Shepard's golf ball.
Notice in the other file, "Async Double", that TRMT never rises--because the port is running at 100% capacity. You could use the high/low ratio of TRMT to determine the RS-232 port's capacity...but please, PLEASE, PLEASE (and pretty please) don't use it to needlessly waste CPU cycles for no benefit.
I stress again that the ONLY required check (which makes no sense to have optional) is to make sure there's room in the send buffer (TXREG) BEFORE sending, by polling TXIF. That's it. (Notice in the "Async Double" waveforms that the TXIF bit perfectly indicates when the next byte can be dumped into TXREG.)
Regarding the #DEFINE to remove the delays, is there any reason you can't reverse that, so the (meaningless) delays have to be intentionally #DEFINEd if desired? (Sorry, I'm getting a little hot over this. . .deep breath, deep breath. . .)
I mean, I can understand, if the HSERSEND routine's evolution looked like this...(no, never mind). If the TXIF poll was added AFTER the initial routine was developed, I can perfectly understand why the delays were added--because without them, most of the characters won't get sent. However, at this point, the delays (and polling TRMT) are the equivalent of Barbie training wheels on a streamlined Olympic racing bike in the Velodrome.
In my case, while I haven't taken an oscilloscope to it, I am pretty certain that by removing everything except the TXIF poll and TXREG loading, I am running near 100% capacity on the serial port at 9600 baud (~1000 characters in just over a second), with no serial transmission errors. That's running the PIC at 1MHz clock rate (doesn't need to run faster)--and did I mention that it is retrieving almost every byte over SPI from the MSSP module? While the EUSART is sending, the CPU starts the MSSP shifting in the next byte, which gets quickly transferred to the EUSART's TXREG--and suddenly the PIC's peripherals are doing all of the heavy work, while the CPU just "feeds" each one. That results in far higher thoroughput than if the CPU has to wait for each peripheral to finish eating breakfast before taking the food to the next.
I wasn't complaining about the comments, as much as sort of asking the reasoning behind adding the delays in the first place (which the comments identified and dated). They were simply evidence towards the issue.
I understand your points and I these are great questions and I not complaining. :-)
Regarding the delays. Historical. These delays are there to support the 1000+ parts we support and we need to maintain backwards compatibility with the existing user programs. Bill Roth has managed this library ( offline at the moment ) was the 'go to man' on this library and the strategy he used was the code as it is today.
So, I think you are recommending the following:
New method:
wait while TXIF = 0
'Write the data byte to the USART.
TXREG = SerData
#IF USART_DELAY <> OFF
'Add USART_DELAY after the byte is sent by the USART module
Wait USART_DELAY
#ENDIF
And, this is key...... a script will automatically set the USART_DELAY to OFF if the TX1REG register exists. So, this way the code is optimised for these USART modules.
The new script: This looks a tad long but this is the only way we can may this work. Essentially, if the user has defined a constant this is used, if not uset then 1 ms... when not the newer usart module.
'Set the default value of the USART_DELAY.
'This will default to 1 ms, if the user has defined a constant then this will be used, and if the register TX1REG exits the value will be set to OFF
#script
'Initally, test for user defined constant
USER_DEFINED_USART_DELAY = 0
'Does the constant exist?
if USART_DELAY then
'if exists set our test value to 1
USER_DEFINED_USART_DELAY = 1
end if
'If the user has NOT defined a constant the following test will be 0
if USER_DEFINED_USART_DELAY = 0 then
'set the default to 1 ms, if the TX1REG exists then set to off
USART_DELAY = "1 ms"
if var(TXREG) then
USART_DELAY = "OFF" 'equates to ZERO!
end if
end if
#endscript
And, new method to ensure we get fully optimised code:This defines a new method that does not have the test for the comport. Therefore, handle USART1 only.
sub HSerSend(In SerData)
asm showdebug Sub_HSerSend|SerData| defined in USART.H standard library
Optimised code for the method would be as follows if we implemented these changes.
~~~
;Overloaded signature: BYTE:
HSERSEND246
;sub_hsersend|serdata| defined in usart.h standard library
SysWaitLoop1
banksel PIR3
btfss PIR3,TX1IF
goto SysWaitLoop1
banksel SERDATA
movf SERDATA,W
banksel TXREG
movwf TXREG
banksel STATUS
return
~~~
The code stays 'as is' for for the older parts with the TXIF register.bit.
I have an fully adapted library here. Do you want to test ? Please. :-)
Last edit: Anobium 2019-08-12
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Regarding the delays. Historical. These delays are there to support the 1000+ parts we support and we need to maintain backwards compatibility with the existing user programs.
To avoid holding the entire project back because of older conventions, I would suggest permanently archiving a "milestone" version of GCB for older projects, with a download link. That way, current and future projects don't get held back by older conventions--and people with older projects can still use them with the older firmware.
That code looks a lot better (for my purposes anyway), and I'd be happy to test it.
One comment: if the purpose of the (now optional) delay is to put a specified pause between RS-232 transmissions (for that ancient hand-crank 300-baud baseball signboard), you will need to poll TRMT to wait for the transmission to finish before adding the delay. Otherwise the default 1mS delay will have completely different results with different baud rates. (It'll slow down all transmissions over "10000" baud; but below that, it'll just slow down the CPU's ability to get the next byte.)
However, I don't understand the following:
The code stays 'as is' for for the older parts with the TXIF register.bit.
and
So, this way the code is optimised for these USART modules.
Unless you can specifically name a certain MCU, if a PIC has an AUSART or EUSART module, it has TXIF--so there should be no special class for "newer parts" vs "older parts." I've programmed RS232 on the older PICs with just an AUSART, and now on the newer ones with a EUSART. I have yet to find a PIC with a AUSART/EUSART that Microchip has a different timing diagram for. And I certainly haven't found one with a USART and no TXIF flag. This fix is not "just for the new PICs"--it's global to ALL PICs with a USART, whether AUSART or EUSART.
That being said, I'm pretty sure that all PICs with a USART have a TXREG, so we should be fine. Just some newer ones have more than one USART module.
Last edit: Siddy 2019-08-12
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
simply define your own sub HSerSend method in your user program and leave the library as it is
Your method will be compiled rather than the library method.
Another idea.
:-)
Last edit: Anobium 2019-08-12
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I could easily do that, but then nobody else would be able to enjoy the benefits of full speed RS-232 transmission. Just trying to help improve an excellent addition to the PIC programming world.
Last edit: Siddy 2019-08-12
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
So has this got any further? Has a new canididate for a HSERSEND been tested?
This is of interest to me as I will likely use the RS232 HW in the future. I think I get the general idea of what Siddy is saying.
I know of one case for my own use where a delay after each character would be useful, but obviously this would be the exception and generally you wouldn't want it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
OK, thanks for the fast response, though I'm a bit confused as I thought defining usart_delay 0 would only work on the proposed new code that you put up? Am I getting mixed up? From what you are saying it seems I can add that define with the current code and will get no delays?
Also, if I WANT to add a delay per character, I can specify a value in ms? - or do I just get the 12ms irrespective?
Thanks for your patience in explaining this :)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
So I've been doing some devving using a PIC16F15345 (though this is by NO means a device-specific issue), and specifically, using the hardware serial port access commands. Anyway, at 9600baud, I should be able to reach 960 bytes per second transmission. I'm sending right around 1000 bytes...but it takes three seconds to send that!
An oscilloscope showed that the comms are NOT back-to-back like they could be. (Note that I am running the PIC at 1MHz, so there may be gaps as the PIC gets the next transmission ready...but not on EVERY character.) However, looking in the compiled ASM output, I found the following:
HSERSEND ;Block before sending (if needed) ;Send byte ;Registers/Bits determined by #samevar at top of file ;if comport = 1 Then decf COMPORT,W btfss STATUS, Z goto ENDIF49 ;HSerSendBlocker ;Wait While TXIF = Off SysWaitLoop3 banksel PIR3 btfss PIR3,TX1IF goto SysWaitLoop3 ;asm showdebug TXREG equals SerData below will assign SerData to TXREG or TXREG1 or U1TXB via the #samevar ;txreg equals serdata below will assign serdata to txreg | txreg1 | txreg via the #samevar ; ;TXREG = SerData banksel SERDATA movf SERDATA,W banksel TXREG movwf TXREG ;Add USART_DELAY After all bits are shifted out ;Wait until TRMT = 1 SysWaitLoop4 btfss TX1STA,TRMT goto SysWaitLoop4 ;Add USART_DELAY After all bits are shifted out ;Wait USART_DELAY movlw 1 movwf SysWaitTempMS clrf SysWaitTempMS_H banksel STATUS call Delay_MS ;exit sub return ;end if ENDIF49 returnNote that I have defined "USART_TX_BLOCKING", which adds the "HSerSendBlocker, wait while TXIF is off." In my (not very) humble opinion, that should be the default setting.
I am at a complete loss to understand the purpose of any lines of code after "movwf TXREG". All they do is completely waste CPU cycles that could be used to "multitask" while the EUSART is sending the byte. Not only does the code wait for the EUSART to finish sending the byte (polling TRMT), it also adds a delay on top of that! No wonder I can't get anywhere close to the 960 byte theoretical maximum @9600baud.
Found the source in "usart.h", with a comment: "'8/2/2010: Added delay after each byte in print." What was the reason? I understand that GCB supports a LARGE array of MCUs, but this doesn't make any sense.
"9/12/2015: Changed default USART_DELAY from 12 ms back to 1 ms - fix very slow USART default!" 12mS after EVERY character? What???!
Looking further through USART.H, I notice that there are NO delays for the AVR implementation, though the "HSerSendBlocker" is still optional, for some strange reason.
I deleted the offending lines out of USART.H. Transmit time for my 1,000 byte transmission? Just over a second (don't have an official means of measuring it.) Talk about an improvement...! (Saved some program memory, to boot.)
Last edit: Siddy 2019-08-10
You finding are very insightful.
You have found code that is not documented when Bill, the developer gentlemen that last adapted the library.
Add #define USART_DELAY OFF to your code and this will optmise.
gives
HSERSEND ;Block before sending (if needed) ;Send byte ;Registers/Bits determined by #samevar at top of file ;if comport = 1 Then decf COMPORT,W btfss STATUS, Z goto ENDIF1 ;HSerSendBlocker ;asm showdebug TXREG equals SerData below will assign SerData to TXREG or TXREG1 or U1TXB via the #samevar ;txreg equals serdata below will assign serdata to txreg | txreg1 | txreg via the #samevar ; ;TXREG = SerData movf SERDATA,W banksel TXREG movwf TXREG ;Add USART_DELAY After all bits are shifted out ;Wait until TRMT = 1 SysWaitLoop1 btfss TX1STA,TRMT goto SysWaitLoop1 ;exit sub banksel STATUS return ;end if ENDIF1 returnThe
btfss TX1STA,TRMTafter themovwf TXREG...The TRMT bit of the TX1STA register indicates the
status of the TSR register. This is a read-only bit. The
TRMT bit is set when the TSR register is empty and is
cleared when a character is transferred to the TSR
register from the TX1REG. The TRMT bit remains clear
until all bits have been shifted out of the TSR register.
No interrupt logic is tied to this bit, so the user has to
poll this bit to determine the TSR status.
Regarding the comments... they can always be improved. We are trying to clean up.
Regarding polling of TRMT: Attached to this post are the timing diagrams from the Microchip PIC16F15345 datasheet, DS40001865B, pages 429-430 (which are by NO means specific to this PIC MCU--it's just what I'm currently working with). One is "Async Single", which shows the waveforms for a single transmit. Notice that TRMT is held LOW until the EUSART has finished sending the entire byte. Waiting for TRMT to rise before returning has the exact code equivalent of sending the RS-232 data via bit-banging--COMPLETELY neutralizing the "transmit" benefit of the EUSART module.
The reason there is no interrupt logic tied to TRMT, is because it is not needed for general data transmission. The only time you would need TRMT is to determine if the I/O port is presently sending data, say, to assert a flow-control line. Or to make sure that the last byte of guidance data has actually been sent before your model rocket launches off on that top secret moon mission to retrieve Alan Shepard's golf ball.
Notice in the other file, "Async Double", that TRMT never rises--because the port is running at 100% capacity. You could use the high/low ratio of TRMT to determine the RS-232 port's capacity...but please, PLEASE, PLEASE (and pretty please) don't use it to needlessly waste CPU cycles for no benefit.
I stress again that the ONLY required check (which makes no sense to have optional) is to make sure there's room in the send buffer (TXREG) BEFORE sending, by polling TXIF. That's it. (Notice in the "Async Double" waveforms that the TXIF bit perfectly indicates when the next byte can be dumped into TXREG.)
Regarding the #DEFINE to remove the delays, is there any reason you can't reverse that, so the (meaningless) delays have to be intentionally #DEFINEd if desired? (Sorry, I'm getting a little hot over this. . .deep breath, deep breath. . .)
I mean, I can understand, if the HSERSEND routine's evolution looked like this...(no, never mind). If the TXIF poll was added AFTER the initial routine was developed, I can perfectly understand why the delays were added--because without them, most of the characters won't get sent. However, at this point, the delays (and polling TRMT) are the equivalent of Barbie training wheels on a streamlined Olympic racing bike in the Velodrome.
In my case, while I haven't taken an oscilloscope to it, I am pretty certain that by removing everything except the TXIF poll and TXREG loading, I am running near 100% capacity on the serial port at 9600 baud (~1000 characters in just over a second), with no serial transmission errors. That's running the PIC at 1MHz clock rate (doesn't need to run faster)--and did I mention that it is retrieving almost every byte over SPI from the MSSP module? While the EUSART is sending, the CPU starts the MSSP shifting in the next byte, which gets quickly transferred to the EUSART's TXREG--and suddenly the PIC's peripherals are doing all of the heavy work, while the CPU just "feeds" each one. That results in far higher thoroughput than if the CPU has to wait for each peripheral to finish eating breakfast before taking the food to the next.
I wasn't complaining about the comments, as much as sort of asking the reasoning behind adding the delays in the first place (which the comments identified and dated). They were simply evidence towards the issue.
Looking the library... this is missing from the USART2 part of the method....
#IF USART_DELAY <> OFF 'Add USART_DELAY After all bits are shifted out Wait USART_DELAY #ENDIFI understand your points and I these are great questions and I not complaining. :-)
Regarding the delays. Historical. These delays are there to support the 1000+ parts we support and we need to maintain backwards compatibility with the existing user programs. Bill Roth has managed this library ( offline at the moment ) was the 'go to man' on this library and the strategy he used was the code as it is today.
So, I think you are recommending the following:
New method:
wait while TXIF = 0 'Write the data byte to the USART. TXREG = SerData #IF USART_DELAY <> OFF 'Add USART_DELAY after the byte is sent by the USART module Wait USART_DELAY #ENDIFAnd, this is key...... a script will automatically set the USART_DELAY to OFF if the TX1REG register exists. So, this way the code is optimised for these USART modules.
The new script: This looks a tad long but this is the only way we can may this work. Essentially, if the user has defined a constant this is used, if not uset then 1 ms... when not the newer usart module.
'Set the default value of the USART_DELAY. 'This will default to 1 ms, if the user has defined a constant then this will be used, and if the register TX1REG exits the value will be set to OFF #script 'Initally, test for user defined constant USER_DEFINED_USART_DELAY = 0 'Does the constant exist? if USART_DELAY then 'if exists set our test value to 1 USER_DEFINED_USART_DELAY = 1 end if 'If the user has NOT defined a constant the following test will be 0 if USER_DEFINED_USART_DELAY = 0 then 'set the default to 1 ms, if the TX1REG exists then set to off USART_DELAY = "1 ms" if var(TXREG) then USART_DELAY = "OFF" 'equates to ZERO! end if end if #endscriptAnd, new method to ensure we get fully optimised code:This defines a new method that does not have the test for the comport. Therefore, handle USART1 only.
Optimised code for the method would be as follows if we implemented these changes.
~~~
;Overloaded signature: BYTE:
HSERSEND246
;sub_hsersend|serdata| defined in usart.h standard library
SysWaitLoop1
banksel PIR3
btfss PIR3,TX1IF
goto SysWaitLoop1
banksel SERDATA
movf SERDATA,W
banksel TXREG
movwf TXREG
banksel STATUS
return
~~~
The code stays 'as is' for for the older parts with the TXIF register.bit.
I have an fully adapted library here. Do you want to test ? Please. :-)
Last edit: Anobium 2019-08-12
To avoid holding the entire project back because of older conventions, I would suggest permanently archiving a "milestone" version of GCB for older projects, with a download link. That way, current and future projects don't get held back by older conventions--and people with older projects can still use them with the older firmware.
That code looks a lot better (for my purposes anyway), and I'd be happy to test it.
One comment: if the purpose of the (now optional) delay is to put a specified pause between RS-232 transmissions (for that ancient hand-crank 300-baud baseball signboard), you will need to poll TRMT to wait for the transmission to finish before adding the delay. Otherwise the default 1mS delay will have completely different results with different baud rates. (It'll slow down all transmissions over "10000" baud; but below that, it'll just slow down the CPU's ability to get the next byte.)
However, I don't understand the following:
and
Unless you can specifically name a certain MCU, if a PIC has an AUSART or EUSART module, it has TXIF--so there should be no special class for "newer parts" vs "older parts." I've programmed RS232 on the older PICs with just an AUSART, and now on the newer ones with a EUSART. I have yet to find a PIC with a AUSART/EUSART that Microchip has a different timing diagram for. And I certainly haven't found one with a USART and no TXIF flag. This fix is not "just for the new PICs"--it's global to ALL PICs with a USART, whether AUSART or EUSART.
That being said, I'm pretty sure that all PICs with a USART have a TXREG, so we should be fine. Just some newer ones have more than one USART module.
Last edit: Siddy 2019-08-12
or,....
simply define your own sub HSerSend method in your user program and leave the library as it is
Your method will be compiled rather than the library method.
Another idea.
:-)
Last edit: Anobium 2019-08-12
I could easily do that, but then nobody else would be able to enjoy the benefits of full speed RS-232 transmission. Just trying to help improve an excellent addition to the PIC programming world.
Last edit: Siddy 2019-08-12
So, can I send you the test library? email me to the mail address I have sent you via a SourceForge personal message.
So has this got any further? Has a new canididate for a HSERSEND been tested?
This is of interest to me as I will likely use the RS232 HW in the future. I think I get the general idea of what Siddy is saying.
I know of one case for my own use where a delay after each character would be useful, but obviously this would be the exception and generally you wouldn't want it.
You have to add
#define usart_delay 0to your user program to resolve this.OK, thanks for the fast response, though I'm a bit confused as I thought defining usart_delay 0 would only work on the proposed new code that you put up? Am I getting mixed up? From what you are saying it seems I can add that define with the current code and will get no delays?
Also, if I WANT to add a delay per character, I can specify a value in ms? - or do I just get the 12ms irrespective?
Thanks for your patience in explaining this :)