MCUdude / MegaCoreX

An Arduino hardware package for ATmega4809, ATmega4808, ATmega3209, ATmega3208, ATmega1609, ATmega1608, ATmega809 and ATmega808
GNU Lesser General Public License v2.1
239 stars 47 forks source link

SoftwareSerial not transmitting on pin D7 #172

Closed ednieuw closed 1 year ago

ednieuw commented 1 year ago

When compiling with MegaCoreX for the Nano Every with Board4809 the string send to pinD7, connected to a Bluetooth HM-10 module with "SoftwareSerial", does not work. When compiling with MegaAVR board the string is transmitted to the Bluetooth HM-10 module.

#include <Wire.h>
#include <SoftwareSerial.h>                        // Arduino for Bluetooth communication
char   sptext[100];  
//------------------------------------------------------------------------------
// Menu
//------------------------------------------------------------------------------  
//0        1         2         3         4         5
//12345678901234567890123456789012345678901234567890  
 char menu[][42] =  {                 
 "String1",                                                
 "String2",
 "String3",
 "String4 Feb 2023" };

//                                                                                            //
//------------------------------------------------------------------------------
// PIN Assigments
//------------------------------------------------------------------------------ 
             #if defined(__AVR_ATmega328P__) || defined(ARDUINO_ARCH_RP2040) || defined (ARDUINO_AVR_NANO_EVERY)|| defined (ARDUINO_AVR_ATmega4809) || defined(ARDUINO_ARCH_SAMD)
enum DigitalPinAssignments {      // Digital hardware constants ATMEGA 328 ----
 RX           = 0,                
 TX           = 1,               
 DCF_PIN      = 2,                // DCFPulse on interrupt  pin
 encoderPinA  = 3,                // right (labeled DT on decoder)on interrupt  pin
 clearButton  = 4,                // switch (labeled SW on decoder)
 LED_PIN      = 5,                // Pin to control colour SK6812/WS2812 LEDs
 BT_TX        = 6,                // Connects to Bluetooth RX
 BT_RX        = 7,                // Connects to Bluetooth TX
 HC_12TX      = 6,                // HC-12 TX Pin  // note RX and TX are reversed compared with a BT Module
 HC_12RX      = 7,                // HC-12 RX Pin 
 encoderPinB  = 8,                // left (labeled CLK on decoder)no interrupt pin  
 secondsPin   = LED_BUILTIN,
 HeartbeatLED = 11,               // PIN10        = 10,                        // PIN 10 
 DCFgood      = 10,               // DCF-signal > 50    
 PIN11        = 11,               // PIN 11
 PIN12        = 12,               // PIN 12             // led = 12,          //
 DCF_LED_Pin  = 13,                // DCF signal
 };

enum AnaloguePinAssignments {     // Analogue hardware constants ----
 EmptyA0      = A0,                // Empty
 EmptyA1      = A1,                // Empty
 PhotoCellPin = A2,                // LDR pin
 OneWirePin   = A3,                // OneWirePin
 SDA_pin      = A4,                // SDA pin
 SCL_pin      = A5,                // SCL pin
 EmptyA6     =  A6,                // Empty
 EmptyA7     =  A7};               // Empty
                             #endif

SoftwareSerial Bluetooth(BT_RX, BT_TX);                           // BT_RX <=> TXD on BT module, BT_TX <=> RXD on BT module

void loop()
{
 SerialCheck();
 BluetoothCheck(); 
}  
//------------------------------------------------------------------------------
// ARDUINO Setup
//------------------------------------------------------------------------------
//                                                                                            //
void setup()
{                                                            
 Serial.begin(9600);                                                                          // Setup the serial port to 9600 baud       
 Tekstprintln("\n*********\nSerial started"); 
 Wire.begin();                                                                                // Start communication with I2C / TWI devices     
 Bluetooth.begin(9600);  
                         #if defined(MEGACOREX)
 Tekstprintln("Compiled with MegaCore X"); 
                         #else 
 Tekstprintln("Compiled with Arduino MegaAVR");   
                         #endif
                          #if defined(ARDUINO_ARCH_RP2040)
 Tekstprintln("Compiled for ARDUINO NANO RP2040");
                          #endif
                          #ifdef ARDUINO_AVR_NANO_EVERY
 Tekstprintln("Compiled for ARDUINO AVR NANO EVERY");
                          #endif
                          #if defined(__AVR_ATmega328P__) 
 Tekstprintln("Compiled for ARDUINO AVR ATmega328P");
                          #endif
                          #if defined(__AVR_ATmega4809__) 
 Tekstprintln("Compiled for ARDUINO_AVR_ATmega4809");
                          #endif                         
 } 

//------------------------------------------------------------------------------
// CLOCK common print routines
//------------------------------------------------------------------------------
//                                                                                            //
void Tekstprint(char const *tekst)
{
 Serial.print(tekst);    
 Bluetooth.print(tekst);  
}

void Tekstprintln(char const *tekst)
{
 strcpy(sptext,tekst);
 strcat(sptext,"\n");          //sprintf(sptext,"%s\n",tekst);
 Tekstprint(sptext);    
}

//------------------------------------------------------------------------------
// CLOCK check for Bluetooth input
//------------------------------------------------------------------------------                           
void BluetoothCheck(void)
{ 
  String  BluetoothString = "";
  char c = 0;
 while (Bluetooth.available()) 
  { 
   c = Bluetooth.read();
   if (c>31 && c<127) BluetoothString += c;
   else c = 0;
   delay(3);
  }
 if (BluetoothString.length()>0) 
   {   
    ReworkInputString(BluetoothString);                                                      // Rework ReworkInputString();
    BluetoothString = "";
   }
}
//------------------------------------------------------------------------------
// CLOCK check for serial input
//------------------------------------------------------------------------------
void SerialCheck(void)
{
 String SerialString = "";
 while (Serial.available())
  {
   char c = Serial.read();  delay(3); 
   if (c>31 && c<127) SerialString += c;                                                      // Allow input from Space - Del
   else c = 0;
  }
 if (SerialString.length()>0)     ReworkInputString(SerialString);                            // Rework ReworkInputString();
 SerialString = "";
}

//------------------------------------------------------------------------------
//  CLOCK Input from Bluetooth or Serial
//------------------------------------------------------------------------------
//                                                                                            //
void ReworkInputString(String InputString)
{
 InputString.trim();                                                                          // Remove trailing spaces
 if (InputString.length()>10) return;                                                         // If string is too long for some reason
 if (InputString.length()< 1) return;                                                         // If string is empty for some reason
 if (InputString[0] > 47 && InputString[0] <123)                                              // If the first charater is a number or letter
  {
  sprintf(sptext,"**** Length fault ****");                                                   // Default message 
  Serial.println(InputString);
  switch (InputString[0]) 
   {
    case 'I':
    case 'i': 
            if (InputString.length() == 1)
            {
             SWversion();
             sptext[0] = 0;                                                                   // Clear sptext
            }
            break;
    }
  }
 Tekstprintln(sptext);                                             
 InputString = "";
 Tekstprintln("");
}

void SWversion(void) 
{ 
 #define FILENAAM (strrchr(__FILE__, '\\') ? strrchr(__FILE__, '\\') + 1 : __FILE__)
 unsigned int i;
 PrintLine(48);
 for (i = 0; i < sizeof(menu) / sizeof(menu[0]); Tekstprintln(menu[i++]));
 PrintLine(48);
 sprintf(sptext,"Software: %s",FILENAAM);                                      
 Tekstprintln(sptext);  // VERSION);  
 sptext[0] = 0;

}
void PrintLine(byte Length)
{
  for (int n=0; n<Length; n++) {Serial.print(F("-"));} Serial.println(); 
}
MCUdude commented 1 year ago

Please provide a bare-minimum sketch that isolates the issue. There's lots of code in your provided example that doesn't have anything to do with the issue I'm trying to isolate

ednieuw commented 1 year ago

This bare minimum is working! Will try to isolate the problem and let you know.

#include <SoftwareSerial.h>    
SoftwareSerial Bluetooth(7, 6); 

void loop()
{
delay(1000);
 Bluetooth.println("Test string to BT"); 
}  

void setup()
{                                                            
 Serial.begin(9600);       
 Bluetooth.begin(9600); 
 Serial.println("\n*********\nSerial started"); 
 Bluetooth.println("test string to BT");                        
 }
ednieuw commented 1 year ago

It is wire.begin If omitted the string is send to the Bluetooth module

#include <Wire.h>
#include <SoftwareSerial.h>    
SoftwareSerial Bluetooth(7, 6); 

void loop()
{
delay(1000);
 Bluetooth.println("Test string to BT"); 
}  

void setup()
{                                                            
 Serial.begin(9600);       
 Bluetooth.begin(9600); 
//  Wire.begin();   // <---- blocks pin 7 output
 Serial.println("\n*********\nSerial started"); 
 Bluetooth.println("test string to BT");                        
 } 
SpenceKonde commented 1 year ago

1) You specifically mentioned pin 7. Is the issue specific to pin 7, or does it impact any pin used as SoftwareSerial TX? 2) What pinout are you using? There are two pin mapping options available for these parts (because the Arduino team got to them before MCUDude did, and as usual, turned what could have been a simple pinout that corresponded directly to the datasheet and was readily usable with other resources without translation into a shitshow and a mess. 3) Why the hell are you using software serial? The 4809 has FOUR HARDWARE SERIAL PORTS. SoftwareSerial sacrifices the ability to make a performant interrupt on any pin (on classic AVR, it does this by defining every pcint vector, on modern AVR it does this by calling attachInterrupt, which is now almost certainly the most evil function in the API.) And it's doing this just to get a single really poor performing (low baud rate) half duplex crapola approximation of a serial port. It's what you use when your back is against the wall, you absolutely need another serial port and there's just nothing you can to to reduce the your serial port requirements (the UART is by far the hardest of the big three interfaces to replace with software),

To be clear though, in none of the pinouts is there a sound reason for starting Wire in master mode to interfere with software serial, so there is absolutely a bug here - Your approach is inefficient and should be a last resort not a first resort - but it certainly should work, and I don't see any plausible route by which it would cause it to not work.

I'd do a sanity check and blink a LED after starting wire... or maybe turn on after starting wire, then after begin()ing the softweare serial, delay for 100ms (to make sure you see the flash) and turn the LED off.

If that passes, move that verification to when you're sending the first data.

That way you confirm that execution is not getting hung up somewhere

ednieuw commented 1 year ago

I made software for the ATMEGA 328 chip and later the Nano. All components are fitted on a PCB. So pin7 is fixed for TX to a Bluetooth module. Because of memory shortage in some projects I use the Nano Every. It also fits on the same PCB lay-out, is cheaper and has more memory. Of course I can make the program specific for the 4809 but that is not convenient for me at this moment. Beside that there is no problem when compiling with the MegaAvr board of Arduino.

MCUdude commented 1 year ago

@ednieuw thanks for providing a neat example and for tracing down the issue.

There is indeed a bug in Wire.begin that specifically affects the Nano Every. It's this code right here. I'll look into it to figure out why this is causing issues.

https://github.com/MCUdude/MegaCoreX/blob/a34d65424f961798435923ce5225b518226fcb02/megaavr/libraries/Wire/src/utility/twi.c#L73-L77

@SpenceKonde is absolutely right though. The Nano Every, when used with MegaCoreX has four(!) hardware serial port. This is obviously much better than having to deal with the limitations a software serial implementation has. But if you're using a Nano Every as a drop-in replacement in an older board where a classic Nano was previously used, your current pinout makes sense.

MCUdude commented 1 year ago

I've just pushed a commit that resolved this issue. It will be available in the next MegaCoreX boards manager release. Meanwhile, you can run Wire.begin() before bluetooth.begin() as a temporary workaround.

ednieuw commented 1 year ago

Top!Thanks for the quick solution!On 19 Feb 2023, at 18:56, Hans @.***> wrote: I've just pushed a commit that resolved this issue. It will be available in the next MegaCoreX boards manager release. Meanwhile, you can run Wire.begin() before bluetooth.begin() as a temporary workaround.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>