MCUdude / MegaCoreX

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

iom3209.h is not directly compatible with iom4809.h, even though it should be. #1

Closed MCUdude closed 5 years ago

MCUdude commented 5 years ago

Since I have two 4809 Curiosity boards at the moment I was able to replace one with an ATmega3209 in order to experiment with it. In various documents Microchip/Atmel states that the ATmega3209 is directly compatible with ATmega4809. The only difference is the flash and ram size.

Well, it turns out that the iom3209.h file is missing many register definitions and support macros. The datasheet states the 3209 has four UARTs, but only two are actually available. Timer3 (TCB3) is not available either, even though it should.

Just to quote the intro of chapter two in the megaAVR-0 manual:

Vertical migration is possible without code modification, as these devices are fully pin and feature compatible.

This is simply not possible with the iom3209.h file shipped with Arduino.

MCUdude commented 5 years ago

It turns out that the files included with Atmel Studio have all this corrected. I've attached the most recent iom3209.h, iom3208.h, iom4808.h and iom4809.h files I could find (ATmega_DFP 1.2.209). These have copyright 2018 in their header, while the files shipped with the Arduino toolchain have copyright 2017. I got these files from Atmel Studio 7 on my work PC.

megaAVR-0.zip

It would be great if the Arduino toolchain was updated with all the new stuff from ATmega_DFP 1.2.209

MCUdude commented 5 years ago

Here's the lib files too. avrxmega3.zip

@per1234 I have no experience with toolchain compilation and patches in general. I just need something that works for this. How can I get my hands on a toolchain for MacOS that has the latest iomXX.h files and fully supports the new ATtinys (Should include ATtiny_DFP 1.3.229)

per1234 commented 5 years ago

It's certainly possible to create your own avr-gcc archive for each OS, host them somewhere on the Internet (could start with using GitHub and then switch to something else if they send you a message that you're using too much bandwidth, which they don't provide any specific abuse threshold numbers for.), and then specify that tool in the Boards Manager JSON file. This is a bit more tricky for manual installation. The ESP8266 and ESP32 cores allow you to install their toolchains for manual installations by running a Python script that downloads the tools and makes sure they are installed to the correct place. I also don't know how difficult the toolchain would be to put together, but Arduino does publish their scripts: https://github.com/arduino/toolchain-avr

However, it would be ideal to get Arduino to fix the issue in their avr-gcc so that you can still use Arduino's avr-gcc. My advice is to open an issue in the arduino/toolchain-avr repository explaining the issue and see whether they will be willing to update the DFP version they are using for the toolchain. Looking at the contents of the repository, it appears they are currently using 1.2.203: https://github.com/arduino/toolchain-avr/blob/d9bf0d7e82540156442ad3da2df8eeaf7f354d42/build.conf#L19

MCUdude commented 5 years ago

Issue opened! https://github.com/arduino/toolchain-avr/issues/60

MCUdude commented 5 years ago

BTW do you know where the avrdude repo that the IDE uses is hosted? The device signature for ATmega3208 and 3209 is wrong in avrdude.conf, but I have no idea where to report it.

per1234 commented 5 years ago

The avrdude build script repository is here: https://github.com/arduino/avrdude-build-script The avrdude source repository they are currently using is here: https://github.com/facchinm/avrdude

I'm not really sure whether changes should be submitted as patches to the avr-build-script repo or as just a straightforward change to the facchinm/avrdude repo. I submitted a PR for a change to the avrdude source code to the facchinm/avrdude repository and it was accepted so I guess that wasn't the wrong thing to do.

It was also reported that the ATtiny402 signature is wrong in the facchinm/avrdude repository: https://github.com/facchinm/avrdude/issues/4#issuecomment-461417963

MCUdude commented 5 years ago

Thanks! I'll compare the signatures for all the new ATtinys too before submitting an issue

per1234 commented 5 years ago

I wonder if this represents some progress towards fixing the toolchain issue?: https://github.com/arduino/toolchain-avr/commit/b235983972887ac66fee36b0753e8a17285726e6

MCUdude commented 5 years ago

Definitly! I'll keep you updated!

MCUdude commented 5 years ago

BTW do you know if there's a good reason for the megaAVR-core to mix spaces and tabs for indentation? I'd definitely prefer if they used (two) spaces like they do with everything else. Didn't you develop a script for converting tabs into spaces?

per1234 commented 5 years ago

do you know if there's a good reason for the megaAVR-core to mix spaces and tabs for indentation?

It's really unfortunate that Arduino never established an official style guide and enforced it. All their code is really a huge mess of inconsistent formatting and the problem continues even in the new projects. I think Arduino megaAVR Boards may have inherited some of this from the existing code it was based on, but it's also possible that the problem continues to grow. I've found that inconsistency breeds more inconsistency. When you're working on some code that has a mixture of formatting styles throughout, it's very difficult to determine what style you should use, and some people probably just give up.

Actually, Arduino has done a decent job of establishing a style for Arduino sketches. They have an AStyle configuration here, which is compatible with the Arduino IDE's Auto Format, but is more strict: https://github.com/arduino/Arduino/blob/master/build/shared/examples_formatter.conf and a script to run it: https://github.com/arduino/Arduino/blob/master/build/shared/examples_formatter.sh I'd love to see the same style applied to all of Arduino's C/C++ code. As the users grow more advanced, they will inevitably start digging into the library and core code and it would be nice if the same formatting style they're already accustomed to from the sketches carried through to that code as well.

There is some discussion of that topic here, but I don't have much hope that it will go anywhere. There have been previous attempts to simply trim trailing whitespace (in the IDE, and Arduino AVR Boards), which should not be controversial at all, but they were never merged. I do understand that these wide ranging changes can cause merge conflicts with a lot of outstanding pull requests, but those should be fairly simple to resolve. The key is to prevent the formatting inconsistencies in the first place, rather than letting them creep in and then going back to fix them later. That's why I think using CI is so great! But if the formatting is to be fixed, the best time to do it is early on, before an accumulation of PRs give an excuse for not fixing it. This is certainly the best time there will ever be for Arduino megaAVR Boards. I am supposed to provide my proposed enhanced AStyle configuration to the issue in the Arduino AVR Boards repository, but I haven't made it a high priority since I haven't had high hopes that it will be used there any time in the near future and I have other volunteer projects that are more promising. However, now that you have me thinking about this opportunity with Arduino megaAVR Boards, I'll try to get that done soon, in case it will be of use.

I never did develop a script to convert tabs into spaces. I do have a check for tabs I use in my CI configurations (which could be adapted to a check for spaces used instead of tabs for indentation if that was the preferred style of a project).

The tricky thing with converting tabs to spaces is that you need to decide how many spaces each tab should equal. When I have done this in your cores to make the files pass the tab check, I manually examined the rest of the file to see if it provides a precedent and if not, I examined other related files. That would be quite difficult to automate. Once I have found the number of spaces I want each tab to equal, it's easy to do the conversion using Notepad++ (Edit > Blank Operations > TAB to Space). The alternative would be to just say "each indentation level is x number of spaces". That is the best for consistency, but will require also changing any files that are already using spaces, but the wrong number of spaces. If I wanted to convert all indentations (tab or space) to a specific number of spaces across many files, I would use AStyle (the tool the Arduino IDE uses for Tools > Auto Format). I plan to eventually add a function to arduino-ci-script that uses AStyle to check whether the code formatting matches the "Arduino style"

MCUdude commented 5 years ago

Another thing that's bothering me is the / / style single line comments. These comments prevent be from commenting out larger chunks of code. I'd would definitly prefer if they (we?) relaced the single line comments with // instead

I'd love to see the same style applied to all of Arduino's C/C++ code. As the users grow more advanced, they will inevitably start digging into the library and core code and it would be nice if the same formatting style they're already accustomed to from the sketches carried through to that code as well.

Me too! I really like the look and feel of MCUdude_corefiles. It is formatted according to AStyle, and single line comments start with // rather than /*.

However I think we should wait with the formatting for now. At the moment I have a few pending PRs, and I really want these and future ones to be merged in order to improve the official core as much as I can before doing a hard fork and continue on my own.

MCUdude commented 5 years ago

I'm still waiting for facchinm to provide a toolchain I can test with. The last one he provided had some issues.

MCUdude commented 5 years ago

Fixed in the latest megaavr boards manager package (1.8.4 or later)