SpenceKonde / DxCore

Arduino core for AVR DA, DB, DD, EA and future DU-series parts - Microchip's latest and greatest AVRs. Library maintainers: Porting help and adviccee is available.
Other
180 stars 47 forks source link

Unified event system library for mega0, tiny0/1/2 and AVR-Dx #52

Closed MCUdude closed 3 years ago

MCUdude commented 3 years ago

Hi!

I have an idea I'd like to discuss with you. As you already know, the event system is pretty powerful and certainly useful if you can wrap your head around it. I wasn't really able to do just that before I really deep dived into the datasheet. And then it appeared to me; It's very simple! A lot simpler than the CCL hardware!

I'm sure other users will find the event system useful, and even easy to use if they have a sleek library to work with. So what do you think about creating a library that works for MegaCoreX, DxCore and megaTinyCore? I can create a working library for MegaCoreX, and you can modify it to work with "your" hardware? According to the datasheets I've read, the event system hardware are very similar across these three "hardware families".

SpenceKonde commented 3 years ago

I have been thinking about this as well!

The AVR-Dx series is a superset of megaAVR 0-series and tinyAVR 2-series (only the tinyAVR 0/1-series is weird (it's weird with a number of the big "modern AVR" features: the PORTMUX registers have non-descriptive names, CCLs can't do interrupts... and the event system; my understanding is that they were also the first to come out, too - so that's why they are weird... by the time the realized "Okay, this sync/async channel distinction is a complicated mess that makes the event system worse", it's worth centralizing the synchronizers" (see, on tinyAVR 0/1, only TCA0 and USART1 IRDA input required a sync channel.... but most other peripherals have their own synchronizers* according to the datasheet (that is, just because it will consume an asych channel doesn't mean it treats it as such, or does under all/most circumstances). So on the next part families, they did those things right.

I think they're trying to do that again with the crazy ADC in the 2-series (I think it's going to be everywhere in the post Dx-generation. The AVR product line is advancing by leaps and bounds these days, it's great times for someone who loves AVR (as I do) - though as the one who is going to have to write the arduino abstraction (and inevitably, a library to expose it's functionality), I'm grateful for the delays!

Anyway, back to event library - the implementation I think, as you said, is straightforward. The problem is the design here; this issue rears it's head with Logic as well - but is a much bigger deal on the event system.

Sure, we want a slightly more friendly library than directly configuring the registers - though frankly it's pretty easy compared to almost every other peripheral...

The problem is that it's extremely awkward to write a reusable library that makes use of the that doesn't fight unnecessarily with any other library that tries to use an event channel (or LUT, for Logic)... And gee, there sure are a lot of things that rely on the event system.. (input capture comes to mind, eg, porting whatever 433 MHz RF receiver library everyone uses, or the TV-remote-control one). There's rarely an actual shortage of channels (or LUTs), but any library that needs one

The "user" of an event library, I feel like would be more library authors that end-users, almost... right? In the case of event channels, whatever was asking Event for a channel would call Event::getChannel(), getChannel(Event::withPinInput(DigitalPinNumber)) or getChannel(generator::rtc_div512)? I can imagine a bunch of approaches... whatever you do, i'd think it'd have to give the caller a pointer to the event channel object? in any event, IMO would be totally cool to say "you need to set the channel generator to something before some other constructor/begin() method/etc might ask for a channel, otherwise no guarantee we won't see that you're not using it and give it to someone else" that way you wouldn't need to track which ones had been checked out... though I think to do a similar thing for Logic you probably would need to keep track of which blocks had been handed out. The more I've thought about this, the more I've realized that Logic really does need a way to allocate LUTs to requesting functions/libraries... (and the Logic class ought to have LogicN.getGenerator() ( just return (EVSYS_CHANNEL0_CCL_LUT0_gc+block_number;).... or maybe the EventChannel class should have setGenerator() and routeTo() methods...

Naturally I'd imagine that the EventChannel (or whatever it gets called) class would have routeTo(ptr,eventuser::event_type) method (which would have a bunch of signatures - you'd pass it a pointer to the thin you wanted to route to and a constant telling it which event input you wanted it routed to (and the event routeTo() method would have a ton of signatures, for pointers to Logic objects, and timers, and so on and so forth.)

That's sort of the lines along which I've been thinking it would need to be. I'm not terribly good with pointers and C++ classes though... and have WAY too much on my plate with the cores currently. Everything is an "on fire" priority, still... and for me getting low power library is the first library I need to work on when I finally can do that - so many people are asking me about how to save power on these things :-( and I have no clue!

MCUdude commented 3 years ago

I was 100% sure I posted the source code I was working on two days ago but turns out I didn't.

This is what I have so far. Note that the Doxygen-style comments in the cpp file is there so "smart editors" like VScode can show you the following information when you hover your mouse pointer over a function.

image

I'm open to feedback!

Event.h:

```c++ #ifndef EVENT_h #define EVENT_h #include // Features present on all generator channels namespace gen { enum generator_t : uint8_t { disable = 0x00, off = 0x00, updi = 0x01, rtc_ovf = 0x06, rtc_cmp = 0x07, ccl0_out = 0x10, ccl1_out = 0x11, ccl2_out = 0x12, ccl3_out = 0x13, ac0_out = 0x20, adc0_ready = 0x24, usart0_xck = 0x60, usart1_xck = 0x61, usart2_xck = 0x62, usart3_xck = 0x63, spi0_sck = 0x68, tca0_ovf_lunf = 0x80, tca0_hunf = 0x81, tca0_cmp0 = 0x84, tca0_cmp1 = 0x85, tca0_cmp2 = 0x86, tcb0_capt = 0xA0, tcb1_capt = 0xA2, tcb2_capt = 0xA4, tcb3_capt = 0xA6, }; }; // Features unique to event generator channel 0 namespace gen0 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div8192 = 0x08, rtc_div4096 = 0x09, rtc_div2048 = 0x0A, rtc_div1024 = 0x0B, pin_pa0 = 0x40, pin_pa1 = 0x41, pin_pa2 = 0x42, pin_pa3 = 0x43, pin_pa4 = 0x44, pin_pa5 = 0x45, pin_pa6 = 0x46, pin_pa7 = 0x47, pin_pb0 = 0x48, pin_pb1 = 0x49, pin_pb2 = 0x4A, pin_pb3 = 0x4B, pin_pb4 = 0x4C, pin_pb5 = 0x4F, pin_pb6 = 0x4E, pin_pb7 = 0x4F, }; }; // Features unique to event generator channel 1 namespace gen1 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div512 = 0x08, rtc_div256 = 0x09, rtc_div128 = 0x0A, rtc_div64 = 0x0B, pin_pa0 = 0x40, pin_pa1 = 0x41, pin_pa2 = 0x42, pin_pa3 = 0x43, pin_pa4 = 0x44, pin_pa5 = 0x45, pin_pa6 = 0x46, pin_pa7 = 0x47, pin_pb0 = 0x48, pin_pb1 = 0x49, pin_pb2 = 0x4A, pin_pb3 = 0x4B, pin_pb4 = 0x4C, pin_pb5 = 0x4F, pin_pb6 = 0x4E, pin_pb7 = 0x4F, }; }; // Features unique to event generator channel 2 namespace gen2 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div8192 = 0x08, rtc_div4096 = 0x09, rtc_div2048 = 0x0A, rtc_div1024 = 0x0B, pin_pc0 = 0x40, pin_pc1 = 0x41, pin_pc2 = 0x42, pin_pc3 = 0x43, pin_pc4 = 0x44, pin_pc5 = 0x45, pin_pc6 = 0x46, pin_pc7 = 0x47, pin_pd0 = 0x48, pin_pd1 = 0x49, pin_pd2 = 0x4A, pin_pd3 = 0x4B, pin_pd4 = 0x4C, pin_pd5 = 0x4F, pin_pd6 = 0x4E, pin_pd7 = 0x4F, }; }; // Features unique to event generator channel 3 namespace gen3 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div512 = 0x08, rtc_div256 = 0x09, rtc_div128 = 0x0A, rtc_div64 = 0x0B, pin_pc0 = 0x40, pin_pc1 = 0x41, pin_pc2 = 0x42, pin_pc3 = 0x43, pin_pc4 = 0x44, pin_pc5 = 0x45, pin_pc6 = 0x46, pin_pc7 = 0x47, pin_pd0 = 0x48, pin_pd1 = 0x49, pin_pd2 = 0x4A, pin_pd3 = 0x4B, pin_pd4 = 0x4C, pin_pd5 = 0x4F, pin_pd6 = 0x4E, pin_pd7 = 0x4F, }; }; // Features unique to event generator channel 4 namespace gen4 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div8192 = 0x08, rtc_div4096 = 0x09, rtc_div2048 = 0x0A, rtc_div1024 = 0x0B, pin_pe0 = 0x40, pin_pe1 = 0x41, pin_pe2 = 0x42, pin_pe3 = 0x43, pin_pe4 = 0x44, pin_pe5 = 0x45, pin_pe6 = 0x46, pin_pe7 = 0x47, pin_pf0 = 0x48, pin_pf1 = 0x49, pin_pf2 = 0x4A, pin_pf3 = 0x4B, pin_pf4 = 0x4C, pin_pf5 = 0x4D, pin_pf6 = 0x4E, }; }; // Features unique to event generator channel 5 namespace gen5 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div512 = 0x08, rtc_div256 = 0x09, rtc_div128 = 0x0A, rtc_div64 = 0x0B, pin_pe0 = 0x40, pin_pe1 = 0x41, pin_pe2 = 0x42, pin_pe3 = 0x43, pin_pe4 = 0x44, pin_pe5 = 0x45, pin_pe6 = 0x46, pin_pe7 = 0x47, pin_pf0 = 0x48, pin_pf1 = 0x49, pin_pf2 = 0x4A, pin_pf3 = 0x4B, pin_pf4 = 0x4C, pin_pf5 = 0x4D, pin_pf6 = 0x4E, }; }; // Features unique to event generator channel 6 namespace gen6 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div8192 = 0x08, rtc_div4096 = 0x09, rtc_div2048 = 0x0A, rtc_div1024 = 0x0B, }; }; // Features unique to event generator channel 7 namespace gen7 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div512 = 0x08, rtc_div256 = 0x09, rtc_div128 = 0x0A, rtc_div64 = 0x0B, }; }; // Generator users namespace user { enum user_t : uint8_t { ccl0_in0 = 0x00, ccl0_in1 = 0x01, ccl1_in0 = 0x02, ccl1_in1 = 0x03, ccl2_in0 = 0x04, ccl2_in1 = 0x05, ccl3_in0 = 0x06, ccl4_in1 = 0x07, evouta = 0x09, evoutb = 0x0A, evoutc = 0x0B, evoutd = 0x0C, evoute = 0x0D, evoutf = 0x0E, usart0 = 0x0F, usart1 = 0x10, usart2 = 0x11, usart3 = 0x12, tca0 = 0x13, tcb0 = 0x14, tcb1 = 0x15, tcb2 = 0x16, tcb3 = 0x17, }; }; class Event { public: static int8_t get_user_channel(user::user_t event_user); Event(uint8_t channel_num, volatile uint8_t &channel_addr); void set_generator(uint8_t generator); uint8_t get_generator(); void set_user(user::user_t event_user); void clear_user(user::user_t event_user); void soft_event(); void start(bool state = true); void stop(); uint8_t get_channel_number(); private: const uint8_t channel_number; volatile uint8_t &channel_address; uint8_t generator_type; }; extern Event Event0; extern Event Event1; extern Event Event2; extern Event Event3; extern Event Event4; extern Event Event5; extern Event Event6; extern Event Event7; #endif ```

Event.cpp:

```c++ #include "Event.h" // Pre-defined objects Event Event0(0, EVSYS_CHANNEL0); Event Event1(1, EVSYS_CHANNEL1); Event Event2(2, EVSYS_CHANNEL2); Event Event3(3, EVSYS_CHANNEL3); Event Event4(4, EVSYS_CHANNEL4); Event Event5(5, EVSYS_CHANNEL5); Event Event6(6, EVSYS_CHANNEL6); Event Event7(7, EVSYS_CHANNEL7); /** * @brief Returns the Event channel number a particular user is connected to * * @param event_user The event user to check * @return int8_t Event channel number. Returns -1 if not connected to any event channel */ static int8_t Event::get_user_channel(user::user_t event_user) { // Figure out what user register to read from to based on the passed parameter volatile uint8_t *user_register = &EVSYS_USERCCLLUT0A + (volatile uint8_t&)event_user; // Return what channel the user is connected to return *user_register - 1; } /** * @brief Construct a new Event object * * @param channel_num Event channel number * @param channel_addr Register address to channel generator */ Event::Event(uint8_t channel_num, volatile uint8_t &channel_addr) : channel_number(channel_num), channel_address(channel_addr) { } /** * @brief Sets a generator for a particular event channel * * @param event_generator Set generator. * Use gen:: for functionality present on all event channels. * Use genN:: for functionality present on channel N. */ void Event::set_generator(uint8_t event_generator) { // Store event generator setting for use in start() and stop() generator_type = event_generator; } /** * @brief Returns the generator type a particular channel has * * @return uint8_t Generator type. Returns 0 if no generator is used */ uint8_t Event::get_generator() { return generator_type; } /** * @brief Sets a user for a particular event channel * * @param event_user The event user to connect to a particular channel */ void Event::set_user(user::user_t event_user) { // Figure out what user register to write to based on the passed parameter volatile uint8_t *user_register = &EVSYS_USERCCLLUT0A + (volatile uint8_t&)event_user; // Connect user to the channel we're working with *user_register = channel_number + 1; } /** * @brief Clears/removed a user from a particular event channel if set * * @param event_user The event user to remove from a particular channel */ void Event::clear_user(user::user_t event_user) { // Figure out what user register to write to based on the passed parameter volatile uint8_t *user_register = &EVSYS_USERCCLLUT0A + (volatile uint8_t&)event_user; // Disconnect from event generator *user_register = 0x00; } /** * @brief Creates a software event for a particular event channel * */ void Event::soft_event() { // Write to the bit that represent the channel in the strobe register EVSYS_STROBE = _BV(channel_number); } /** * @brief Starts the event generator for a particular event channel * * @param state Optional parameter. Defaults to true */ void Event::start(bool state) { if (state) { // Write event generator setting to EVSYS.CHANNELn register channel_address = generator_type; } else { // Disable event generator channel_address = gen::disable; } } /** * @brief Stops the event generator for a particular event channel * */ void Event::stop() { start(false); } /** * @brief Returns the event channel number in use * * @return uint8_t Event channel number */ uint8_t Event::get_channel_number() { return channel_number; } ```

Example sketch:

```c++ #include void setup() { Serial3.begin(115200); Event4.set_generator(gen4::pin_pf6); // You can connect upto 23 users Event4.set_user(user::evoute); Event4.set_user(user::evoutf); Event4.start(); } void loop() { Serial3.printf("We're currently using Event channel %d\n", Event4.get_channel_number()); Serial3.printf("We're currently using generator 0x%02x\n", Event4.get_generator()); Serial3.printf("Event user evoutf is connected to channel %d\n", Event::get_user_channel(user::evoutf)); while(1); } ```

First, I'll like to point out that I want the library to "feel" the same as Logic and Comparator does. This means object-oriented, predefined objects and the user doesn't have to deal with references or pointers.

Sure, we want a slightly more friendly library than directly configuring the registers - though frankly it's pretty easy compared to almost every other peripheral...

Yup, the configuration itself is easy, it took me a while to realize how the event user registers actually works since it's a bit different from what I'm used to. It's also confusing that different event channels (odd, even) have different functionality. This library together will make this a much more pleasant experience. If you, on top of the library, have a good text editor/IDE such as VSCode/Platformio, you can just type gen4:: and get a list of all the unique features that channel has.

The problem is that it's extremely awkward to write a reusable library that makes use of the that doesn't fight unnecessarily with any other library that tries to use an event channel (or LUT, for Logic)... And gee, there sure are a lot of things that rely on the event system.. (input capture comes to mind, eg, porting whatever 433 MHz RF receiver library everyone uses, or the TV-remote-control one). There's rarely an actual shortage of channels (or LUTs), but any library that needs one

The Event system library should be fully independent and shouldn't touch other registers than it needs to set up the requested generator and the user.

MCUdude commented 3 years ago

I've now added AVR-DA and AVR-DB support to the library. I haven't really bothered with tiny0/tiny1 support, since the event system is so different. Looks like tiny2 is good though. Anyways, here it is.

Here is the example sketch that shows how to set up a channel, and where we read out what kind of generator the channel is using, and what even channel a particular user is connected to.

Note that I have "grouped" all AVR-DAs and AVR-DBs without checking whenever a peripheral is physically available on that particular chip. For instance, it is possible to use EVOUTG (port G) with the 28-pin AVR-DA package. It requires a bit of work from my side because I don't know the DA's and DB's that well that I remember all the peripherals in my head.

I've also not implemented PORTMUX pin swapping of the EVOUTx pin.

I'm still open to feedback!

#include <Event.h>

void setup()
{
  Serial1.begin(115200);
  delay(1000);
  Event2.set_generator(gen2::pin_pc7); // Use pin PC7 as event generator
  Event2.set_user(user::evoutc);       // Connect generator to EVOUT C (PC2)

  // You can connect upto 23 users
  //Event2.set_user(user::evoute);
  Event2.start();

  Serial1.printf("We're currently using Event channel %d\n",       Event2.get_channel_number());
  Serial1.printf("We're currently using generator 0x%02x\n",       Event2.get_generator());
  Serial1.printf("Event user evoutf is connected to channel %d\n", Event::get_user_channel(user::evoutc));

}

void loop()
{

}

Event.h

```c++ #ifndef EVENT_H #define EVENT_H #include // Helper macros to prevent us from having to check for every possible target #if defined(__AVR_ATmega4809__) || defined(__AVR_ATmega4808__) \ || defined(__AVR_ATmega4809__) || defined(__AVR_ATmega4808__) \ || defined(__AVR_ATmega4809__) || defined(__AVR_ATmega4808__) \ || defined(__AVR_ATmega4809__) || defined(__AVR_ATmega4808__) #define MEGAAVR_0 #elif defined(__AVR_AVR128DA64__) || defined(__AVR_AVR128DA48__) \ || defined(__AVR_AVR128DA32__) || defined(__AVR_AVR128DA28__) \ || defined(__AVR_AVR64DA64__) || defined(__AVR_AVR64DA48__) \ || defined(__AVR_AVR64DA32__) || defined(__AVR_AVR64DA28__) \ || defined(__AVR_AVR32DA48__) || defined(__AVR_AVR32DA32__) \ || defined(__AVR_AVR32DA28__) #define AVR_DA #elif defined(__AVR_AVR128DB64__) || defined(__AVR_AVR128DB48__) \ || defined(__AVR_AVR128DB32__) || defined(__AVR_AVR128DB28__) \ || defined(__AVR_AVR64DB64__) || defined(__AVR_AVR64DB48__) \ || defined(__AVR_AVR64DB32__) || defined(__AVR_AVR64DB28__) \ || defined(__AVR_AVR32DB48__) || defined(__AVR_AVR32DB32__) \ || defined(__AVR_AVR32DB28__) #define AVR_DB #endif // Features present on all generator channels namespace gen { enum generator_t : uint8_t { disable = 0x00, off = 0x00, updi_synch = 0x01, rtc_ovf = 0x06, rtc_cmp = 0x07, ccl0_out = 0x10, ccl1_out = 0x11, ccl2_out = 0x12, ccl3_out = 0x13, ac0_out = 0x20, adc0_ready = 0x24, usart0_xck = 0x60, usart1_xck = 0x61, usart2_xck = 0x62, usart3_xck = 0x63, spi0_sck = 0x68, tca0_ovf_lunf = 0x80, tca0_hunf = 0x81, tca0_cmp0 = 0x84, tca0_cmp1 = 0x85, tca0_cmp2 = 0x86, tcb0_capt = 0xA0, tcb1_capt = 0xA2, tcb2_capt = 0xA4, tcb3_capt = 0xA6, #if defined(AVR_DA) || defined(AVR_DB) ccl4_out = 0x14, ccl5_out = 0x15, ac1_out = 0x21, ac2_out = 0x22, zcd0_out = 0x30, zcd1_out = 0x31, zcd2_out = 0x32, usart4_xck = 0x64, usart5_xck = 0x65, spi1_sck = 0x69, tca1_ovf_lunf = 0x88, tca1_hunf = 0x89, tca1_cmp0 = 0x8C, tca1_cmp1 = 0x8D, tca1_cmp2 = 0x8E, tcb0_ovf = 0xA1, tcb1_ovf = 0xA3, tcb2_ovf = 0xA5, tcb3_ovf = 0xA7, tcb4_capt = 0xA8, tcb4_ovf = 0xA9, tcd0_cmpbclr = 0xB0, tcd0_cmpaset = 0xB1, tcd0_cmpbset = 0xB2, tcd0_progev = 0xB3, #endif #if defined(AVR_DB) mvio_ok = 0x05, opamp0_ready = 0x34, opamp1_ready = 0x35, opamp2_ready = 0x36, #endif }; }; // Features unique to event generator channel 0 #if defined(EVSYS_CHANNEL0) namespace gen0 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div8192 = 0x08, rtc_div4096 = 0x09, rtc_div2048 = 0x0A, rtc_div1024 = 0x0B, pin_pa0 = 0x40, pin_pa1 = 0x41, pin_pa2 = 0x42, pin_pa3 = 0x43, pin_pa4 = 0x44, pin_pa5 = 0x45, pin_pa6 = 0x46, pin_pa7 = 0x47, pin_pb0 = 0x48, pin_pb1 = 0x49, pin_pb2 = 0x4A, pin_pb3 = 0x4B, pin_pb4 = 0x4C, pin_pb5 = 0x4F, pin_pb6 = 0x4E, pin_pb7 = 0x4F, }; }; #endif // Features unique to event generator channel 1 #if defined(EVSYS_CHANNEL1) namespace gen1 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div512 = 0x08, rtc_div256 = 0x09, rtc_div128 = 0x0A, rtc_div64 = 0x0B, pin_pa0 = 0x40, pin_pa1 = 0x41, pin_pa2 = 0x42, pin_pa3 = 0x43, pin_pa4 = 0x44, pin_pa5 = 0x45, pin_pa6 = 0x46, pin_pa7 = 0x47, pin_pb0 = 0x48, pin_pb1 = 0x49, pin_pb2 = 0x4A, pin_pb3 = 0x4B, pin_pb4 = 0x4C, pin_pb5 = 0x4F, pin_pb6 = 0x4E, pin_pb7 = 0x4F, }; }; #endif // Features unique to event generator channel 2 #if defined(EVSYS_CHANNEL2) namespace gen2 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div8192 = 0x08, rtc_div4096 = 0x09, rtc_div2048 = 0x0A, rtc_div1024 = 0x0B, pin_pc0 = 0x40, pin_pc1 = 0x41, pin_pc2 = 0x42, pin_pc3 = 0x43, pin_pc4 = 0x44, pin_pc5 = 0x45, pin_pc6 = 0x46, pin_pc7 = 0x47, pin_pd0 = 0x48, pin_pd1 = 0x49, pin_pd2 = 0x4A, pin_pd3 = 0x4B, pin_pd4 = 0x4C, pin_pd5 = 0x4F, pin_pd6 = 0x4E, pin_pd7 = 0x4F, }; }; #endif // Features unique to event generator channel 3 #if defined(EVSYS_CHANNEL3) namespace gen3 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div512 = 0x08, rtc_div256 = 0x09, rtc_div128 = 0x0A, rtc_div64 = 0x0B, pin_pc0 = 0x40, pin_pc1 = 0x41, pin_pc2 = 0x42, pin_pc3 = 0x43, pin_pc4 = 0x44, pin_pc5 = 0x45, pin_pc6 = 0x46, pin_pc7 = 0x47, pin_pd0 = 0x48, pin_pd1 = 0x49, pin_pd2 = 0x4A, pin_pd3 = 0x4B, pin_pd4 = 0x4C, pin_pd5 = 0x4F, pin_pd6 = 0x4E, pin_pd7 = 0x4F, }; }; #endif // Features unique to event generator channel 4 #if defined(EVSYS_CHANNEL4) namespace gen4 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div8192 = 0x08, rtc_div4096 = 0x09, rtc_div2048 = 0x0A, rtc_div1024 = 0x0B, pin_pe0 = 0x40, pin_pe1 = 0x41, pin_pe2 = 0x42, pin_pe3 = 0x43, pin_pe4 = 0x44, pin_pe5 = 0x45, pin_pe6 = 0x46, pin_pe7 = 0x47, pin_pf0 = 0x48, pin_pf1 = 0x49, pin_pf2 = 0x4A, pin_pf3 = 0x4B, pin_pf4 = 0x4C, pin_pf5 = 0x4D, pin_pf6 = 0x4E, }; }; #endif // Features unique to event generator channel 5 #if defined(EVSYS_CHANNEL5) namespace gen5 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div512 = 0x08, rtc_div256 = 0x09, rtc_div128 = 0x0A, rtc_div64 = 0x0B, pin_pe0 = 0x40, pin_pe1 = 0x41, pin_pe2 = 0x42, pin_pe3 = 0x43, pin_pe4 = 0x44, pin_pe5 = 0x45, pin_pe6 = 0x46, pin_pe7 = 0x47, pin_pf0 = 0x48, pin_pf1 = 0x49, pin_pf2 = 0x4A, pin_pf3 = 0x4B, pin_pf4 = 0x4C, pin_pf5 = 0x4D, pin_pf6 = 0x4E, }; }; #endif // Features unique to event generator channel 6 #if defined(EVSYS_CHANNEL6) namespace gen6 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div8192 = 0x08, rtc_div4096 = 0x09, rtc_div2048 = 0x0A, rtc_div1024 = 0x0B, #if defined(AVR_DA) || defined(AVR_DB) pin_pg0 = 0x40, pin_pg1 = 0x41, pin_pg2 = 0x42, pin_pg3 = 0x43, pin_pg4 = 0x44, pin_pg5 = 0x45, pin_pg6 = 0x46, pin_pg7 = 0x47, #endif }; }; #endif // Features unique to event generator channel 7 #if defined(EVSYS_CHANNEL7) namespace gen7 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div512 = 0x08, rtc_div256 = 0x09, rtc_div128 = 0x0A, rtc_div64 = 0x0B, #if defined(AVR_DA) || defined(AVR_DB) pin_pg0 = 0x40, pin_pg1 = 0x41, pin_pg2 = 0x42, pin_pg3 = 0x43, pin_pg4 = 0x44, pin_pg5 = 0x45, pin_pg6 = 0x46, pin_pg7 = 0x47, #endif }; }; #endif // Features unique to event generator channel 8 #if defined(EVSYS_CHANNEL8) namespace gen8 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div8192 = 0x08, rtc_div4096 = 0x09, rtc_div2048 = 0x0A, rtc_div1024 = 0x0B, }; }; #endif // Features unique to event generator channel 9 #if defined(EVSYS_CHANNEL9) namespace gen9 { enum generator_t : uint8_t { disable = 0x00, off = 0x00, rtc_div512 = 0x08, rtc_div256 = 0x09, rtc_div128 = 0x0A, rtc_div64 = 0x0B, }; }; #endif // Generator users namespace user { enum user_t : uint8_t { ccl0_event_a = 0x00, ccl0_event_b = 0x01, ccl1_event_a = 0x02, ccl1_event_b = 0x03, ccl2_event_a = 0x04, ccl2_event_b = 0x05, ccl3_event_a = 0x06, ccl3_event_b = 0x07, #if defined(MEGAAVR_0) adc0_start = 0x08, evouta = 0x09, evoutb = 0x0A, evoutc = 0x0B, evoutd = 0x0C, evoute = 0x0D, evoutf = 0x0E, usart0_irda = 0x0F, usart1_irda = 0x10, usart2_irda = 0x11, usart3_irda = 0x12, tca0 = 0x13, tcb0 = 0x14, tcb1 = 0x15, tcb2 = 0x16, tcb3 = 0x17, #endif #if defined(AVR_DA) ccl4_event_a = 0x08, ccl4_event_b = 0x09, ccl5_event_a = 0x0A, ccl5_event_b = 0x0B, adc0_start = 0x0C, ptc_start = 0x0D, evouta = 0x0E, evoutb = 0x0F, evoutc = 0x10, evoutd = 0x11, evoute = 0x12, evoutf = 0x13, evoutg = 0x14, usart0_irda = 0x15, usart1_irda = 0x16, usart2_irda = 0x17, usart3_irda = 0x18, usart4_irda = 0x19, usart5_irda = 0x1A, tca0_cnta = 0x1B, tca0_cntb = 0x1C, tca1_cnta = 0x1D, tca1_cntb = 0x1E, tcb0_capt = 0x1F, tcb0_cnt = 0x20, tcb1_capt = 0x21, tcb1_cnt = 0x22, tcb2_capt = 0x23, tcb2_cnt = 0x24, tcb3_capt = 0x25, tcb3_cnt = 0x26, tcb4_capt = 0x27, tcb4_cnt = 0x28, tcd0_in_a = 0x29, tcd0_in_b = 0x2A, #endif #if defined(AVR_DB) ccl4_event_a = 0x08, ccl4_event_b = 0x09, ccl5_event_a = 0x0A, ccl5_event_b = 0x0B, adc0_start = 0x0C, evouta = 0x0D, evoutb = 0x0E, evoutc = 0x0F, evoutd = 0x10, evoute = 0x11, evoutf = 0x12, evoutg = 0x13, usart0_irda = 0x14, usart1_irda = 0x15, usart2_irda = 0x16, usart3_irda = 0x17, usart4_irda = 0x18, usart5_irda = 0x19, tca0_cnta = 0x1A, tca0_cntb = 0x1B, tca1_cnta = 0x1C, tca1_cntb = 0x1D, tcb0_capt = 0x1E, tcb0_cnt = 0x1F, tcb1_capt = 0x20, tcb1_cnt = 0x21, tcb2_capt = 0x22, tcb2_cnt = 0x23, tcb3_capt = 0x24, tcb3_cnt = 0x25, tcb4_capt = 0x26, tcb4_cnt = 0x27, tcd0_in_a = 0x28, tcd0_in_b = 0x29, opamp0_enable = 0x2A, opamp0_disable = 0x2B, opamp0_dump = 0x2C, opamp0_drive = 0x2D, opamp1_enable = 0x2E, opamp1_disable = 0x2F, opamp1_dump = 0x30, opamp1_drive = 0x31, opamp2_enable = 0x32, opamp2_disable = 0x33, opamp2_dump = 0x34, opamp2_drive = 0x35, #endif }; }; class Event { public: Event(uint8_t channel_num, volatile uint8_t &channel_addr); static int8_t get_user_channel(user::user_t event_user); uint8_t get_channel_number(); void set_generator(uint8_t generator); uint8_t get_generator(); void set_user(user::user_t event_user); void clear_user(user::user_t event_user); void soft_event(); void start(bool state = true); void stop(); private: const uint8_t channel_number; // Holds the event generator channel number volatile uint8_t &channel_address; // Reference to the event channel address uint8_t generator_type; // Generator type the event channel is using }; #if defined(EVSYS_CHANNEL0) extern Event Event0; #endif #if defined(EVSYS_CHANNEL1) extern Event Event1; #endif #if defined(EVSYS_CHANNEL2) extern Event Event2; #endif #if defined(EVSYS_CHANNEL3) extern Event Event3; #endif #if defined(EVSYS_CHANNEL4) extern Event Event4; #endif #if defined(EVSYS_CHANNEL5) extern Event Event5; #endif #if defined(EVSYS_CHANNEL6) extern Event Event6; #endif #if defined(EVSYS_CHANNEL7) extern Event Event7; #endif #if defined(EVSYS_CHANNEL8) extern Event Event8; #endif #if defined(EVSYS_CHANNEL9) extern Event Event9; #endif #endif // EVENT_H ```

Event.cpp

```c++ #include "Event.h" // Pre-defined objects #if defined(EVSYS_CHANNEL0) Event Event0(0, EVSYS_CHANNEL0); #endif #if defined(EVSYS_CHANNEL1) Event Event1(1, EVSYS_CHANNEL1); #endif #if defined(EVSYS_CHANNEL2) Event Event2(2, EVSYS_CHANNEL2); #endif #if defined(EVSYS_CHANNEL3) Event Event3(3, EVSYS_CHANNEL3); #endif #if defined(EVSYS_CHANNEL4) Event Event4(4, EVSYS_CHANNEL4); #endif #if defined(EVSYS_CHANNEL5) Event Event5(5, EVSYS_CHANNEL5); #endif #if defined(EVSYS_CHANNEL6) Event Event6(6, EVSYS_CHANNEL6); #endif #if defined(EVSYS_CHANNEL7) Event Event7(7, EVSYS_CHANNEL7); #endif #if defined(EVSYS_CHANNEL8) Event Event8(8, EVSYS_CHANNEL8); #endif #if defined(EVSYS_CHANNEL9) Event Event9(9, EVSYS_CHANNEL9); #endif /** * @brief Construct a new Event object * * @param channel_num Event channel number * @param channel_addr Register address to channel generator */ Event::Event(uint8_t channel_num, volatile uint8_t &channel_addr) : channel_number(channel_num), channel_address(channel_addr) { } /** * @brief Returns the Event channel number a particular user is connected to * * @param event_user The event user to check * @return int8_t Event channel number. Returns -1 if not connected to any event channel */ int8_t Event::get_user_channel(user::user_t event_user) { // Figure out what user register to read from to based on the passed parameter volatile uint8_t *user_register = &EVSYS_USERCCLLUT0A + (volatile uint8_t&)event_user; // Return what channel the user is connected to return *user_register - 1; } /** * @brief Returns the event channel number in use * * @return uint8_t Event channel number */ uint8_t Event::get_channel_number() { return channel_number; } /** * @brief Sets a generator for a particular event channel * * @param event_generator Set generator. * Use gen:: for functionality present on all event channels. * Use genN:: for functionality present on channel N. */ void Event::set_generator(uint8_t event_generator) { // Store event generator setting for use in start() and stop() generator_type = event_generator; } /** * @brief Returns the generator type a particular channel has * * @return uint8_t Generator type. Returns 0 if no generator is used */ uint8_t Event::get_generator() { return generator_type; } /** * @brief Sets a user for a particular event channel * * @param event_user The event user to connect to a particular channel */ void Event::set_user(user::user_t event_user) { // Figure out what user register to write to based on the passed parameter volatile uint8_t *user_register = &EVSYS_USERCCLLUT0A + (volatile uint8_t&)event_user; // Connect user to the channel we're working with *user_register = channel_number + 1; } /** * @brief Clears/removed a user from a particular event channel if set * * @param event_user The event user to remove from a particular channel */ void Event::clear_user(user::user_t event_user) { // Figure out what user register to write to based on the passed parameter volatile uint8_t *user_register = &EVSYS_USERCCLLUT0A + (volatile uint8_t&)event_user; // Disconnect from event generator *user_register = 0x00; } /** * @brief Creates a software event for a particular event channel * */ void Event::soft_event() { // Write to the bit that represent the channel in the strobe register #if defined(MEGAAVR_0) EVSYS_STROBE = (1 << channel_number); #elif defined(AVR_DA) || defined(AVR_DB) if(channel_number < 8) EVSYS_SWEVENTA = (1 << channel_number); else EVSYS_SWEVENTB = (1 << (channel_number - 8)); #endif } /** * @brief Starts the event generator for a particular event channel * * @param state Optional parameter. Defaults to true */ void Event::start(bool state) { if (state) { // Write event generator setting to EVSYS_CHANNELn register channel_address = generator_type; } else { // Disable event generator channel_address = gen::disable; } } /** * @brief Stops the event generator for a particular event channel * */ void Event::stop() { start(false); } ```
SpenceKonde commented 3 years ago

Thanks for this - I'm going to be taking a deeper look at this soon, but my initial impression is that it looks good. While I do think a "event channel manager" of some sort will be needed, that can just as well be a separate library and problem for another day (it certainly opens up a whole case of canned worms - ugh, thinking about it gives me a headache).

My main question is, how do you envision passing around references to logic channels to happen? You have an Eventn.get_channel_number() method to get the event channel number, which would only have application in a case where one is referring to the logic channel as something other than, say, Logic5. This may be my inexperience with classes speaking here, but I'm unclear on what the right way for that to happen would be.... Should there also be a function to go the other way, that is, to turn a number into an instance of Event?

If the goal is to free people from having to manually manipulate registers, this would be the place to put a way to set the PORTMUX for event output pin - though I recognize that it doesn't have an obvious "place to go" in this library, unlike in Logic...

I can do the conversion for the tiny 0/1-series parts (which seem to have the "beta" version of the event system... and the PORTMUX, for that matter) - they are a bit uglier, heh...

MCUdude commented 3 years ago

My main question is, how do you envision passing around references to logic channels to happen? You have an Eventn.get_channel_number() method to get the event channel number, which would only have application in a case where one is referring to the logic channel as something other than, say, Logic5. This may be my inexperience with classes speaking here, but I'm unclear on what the right way for that to happen would be.... Should there also be a function to go the other way, that is, to turn a number into an instance of Event?

Not 100% I understand the issue you're trying to express, but I'll give it a try. The Logic and Event libraries and their objects live completely separate lives. It's up to the user to tie them together. The user "manually" selects what event channel to use, what generator it should have, and its users. If one of the users is a CCL input, that's fine but doesn't really affect the Logic library. The user still has to use Logic0.input1 = in::event_a in order to "tie" them together. BTW I see now that in::event_a isn't really a good name. We should use numbers instead: in::event0 or in::event_0. Well, we already did.

The EventN.get_channel_number() function (popularly referred to as methods in other OO languages such as Java) is just a dumb function which only outputs the channel number, in case this is needed for some reason.

if you want to create a function that takes an Event object as a parameter to manipulate it there, it can be done like so:


void manipulate_event(Event &myEvent)
{
  if(myEvent.get_channel_number() == 1)
    myEvent.set_generator(gen1::pin_pa0);
  else if(myEvent.get_channel_number() == 2)
    myEvent.set_generator(gen2::pin_pc0);
}
SpenceKonde commented 3 years ago

Oh, if you can specify an event object as a parameter like that, it solves the problem

MCUdude commented 3 years ago

Oh, if you can specify an event object as a parameter like that, it solves the problem

You can do this with any object really. I've always done it with class and struct objects. Very useful!

But no matter what I/we do, this library would be a bit more "fiddly" than say the Comparator library since this is more closely coupled to other peripherals with weird features (like USART irda). We'll have to provide some good examples, and some good documentation, so the end-user understands what this is all about.

MCUdude commented 3 years ago

I've now published the source code for the library here: https://github.com/MCUdude/MegaCoreX/tree/master/megaavr/libraries/Event

I still have to proof-read the README and add some examples, but I think the library itself is doing what it should.

SpenceKonde commented 3 years ago

Damn! Very nice! Think it's ready to bring into DxCore then? Expecting to push out 1.3.0 as soon as I get confirmation that I fixed the toolchain issue for linux users (which I think I have)

MCUdude commented 3 years ago

Feel free to submit a PR if you find any typos, poor formulation or vital information are missing! I will use the next couple of days to create some relevant examples of megaAVR0's. When I'm all done, I'm publishing a new MegaCoreX release. I hope we can cooperate the same way as with the other libraries; we share the source files but provide slightly different examples and documentation to match the actual hardware in use.

BTW does the boards manager script(s) work well for you?

SpenceKonde commented 3 years ago

Seems to be working well now - had to add a call to sed to change the path for python, otherwise pyupdi programming was hosed (for 2.2.0 and 2.2.1 I did the change by hand - cant use same path for python tools for both manual and board manager - like the esp8266 core). But that is in, and all is well.

I have new toolchain package that I'm waiting to hear back from testers about use on linux; I think that is the last thing I am waiting on for 1.3.0! Which would be the least behind I would feel since the summer!


Spence Konde Azzy’S Electronics

New products! Check them out at tindie.com/stores/DrAzzy GitHub: github.com/SpenceKonde ATTinyCore: Arduino support for almost every ATTiny microcontroller Contact: spencekonde@gmail.com

On Sun, Jan 17, 2021, 05:20 Hans notifications@github.com wrote:

Feel free to submit a PR if you find any typos, poor formulation or vital information are missing! I will use the next couple of days to create some relevant examples of megaAVR0's. When I'm all done, I'm publishing a new MegaCoreX release. I hope we can cooperate the same way as with the other libraries; we share the source files but provide slightly different examples and documentation to match the actual hardware in use.

BTW does the boards manager script(s) work well for you?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/SpenceKonde/DxCore/issues/52#issuecomment-761766497, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTXEW2WZF2XZZET5UC4QNDS2K2VBANCNFSM4VOHSQSA .

MCUdude commented 3 years ago

I think I've finished the Event library. I've provided three basic examples, so it should be enough for users to get started. I've also implemented pin swapping for the EVOUT user. It did require some bit manipulating magic, but it does work as it should. Do you have other ideas for examples I can provide?

Seems to be working well now - had to add a call to sed to change the path for python, otherwise pyupdi programming was hosed (for 2.2.0 and 2.2.1 I did the change by hand - cant use same path for python tools for both manual and board manager - like the esp8266 core). But that is in, and all is well.

Great to hear it does the job. I've never really used sed, I've been an awk guy ever since I created a large-ish script for a DSP library I host.

MCUdude commented 3 years ago

I just saw your latest commit. Great work!

I have an idea though. As you probably already have realized, the code gets less and less readable when we add all these ifdefs. What if we format the code like this instead: At least all relevant code lines have the same indents, and all the equal signs line up:


// Generator users
namespace user
{
  enum user_t : uint8_t
  {
#if defined(__AVR_ATmegax08__) || defined(__AVR_ATmegax09__)
    ccl0_event_a   = 0x00,
    ccl0_event_b   = 0x01,
    ccl1_event_a   = 0x02,
    ccl1_event_b   = 0x03,
    ccl2_event_a   = 0x04,
    ccl2_event_b   = 0x05,
    ccl3_event_a   = 0x06,
    ccl3_event_b   = 0x07,
    adc0_start     = 0x08,
    evouta_pin_pa2 = 0x09,
#if defined(__AVR_ATmegax09__)
    evoutb_pin_pb2 = 0x0A,
#endif
    evoutc_pin_pc2 = 0x0B,
    evoutd_pin_pd2 = 0x0C,
#if defined(__AVR_ATmegax09__)
    evoute_pin_pe2 = 0x0D,
  #endif
    evoutf_pin_pf2 = 0x0E,
    usart0_irda    = 0x0F,
    usart1_irda    = 0x10,
    usart2_irda    = 0x11,
#if defined(USART3)
    usart3_irda    = 0x12,
#endif
    tca0           = 0x13,
    tcb0           = 0x14,
    tcb1           = 0x15,
    tcb2           = 0x16,
#if defined(TCB3)
    tcb3           = 0x17,
#endif
    // "Unofficial" user generators. Uses EVOUT, but swaps the output pin using PORTMUX
    evouta_pin_pa7 = 0x89,
#if defined(__AVR_ATmegax09__)
    evoutc_pin_pc7 = 0x8B,
#endif
    evoutd_pin_pd7 = 0x8C,
#endif
#if defined(__AVR_DA__)
    ccl0_event_a   = 0x00,
    ccl0_event_b   = 0x01,
    ccl1_event_a   = 0x02,
    ccl1_event_b   = 0x03,
    ccl2_event_a   = 0x04,
    ccl2_event_b   = 0x05,
    ccl3_event_a   = 0x06,
    ccl3_event_b   = 0x07,
#if defined(LUT4)
    ccl4_event_a   = 0x08,
    ccl4_event_b   = 0x09,
    ccl5_event_a   = 0x0A,
    ccl5_event_b   = 0x0B,
#endif
    adc0_start     = 0x0C,
    ptc_start      = 0x0D,
    evouta_pin_pa2 = 0x0E,
#if defined(Dx_48_PINS) || defined(Dx_64_PINS)
    evoutb_pin_pb2 = 0x0F,
#endif
    evoutc_pin_pc2 = 0x10,
    evoutd_pin_pd2 = 0x11,
#if defined(Dx_48_PINS) || defined(Dx_64_PINS)
    evoute_pin_pe2 = 0x12,
#endif
#if !defined(Dx_28_PINS)
    evoutf_pin_pf2 = 0x13,
#endif
#if defined (Dx_64_PINS)
    evoutg_pin_pg2 = 0x14,
#endif
    usart0_irda    = 0x15,
    usart1_irda    = 0x16,
    usart2_irda    = 0x17,
#if defined(USART5)
    usart3_irda    = 0x18,
#endif
#if defined(USART4)
    usart4_irda    = 0x19,
#endif
#if defined(USART5)
    usart5_irda    = 0x1A,
#endif
    tca0_cnta      = 0x1B,
    tca0_cntb      = 0x1C,
    tca1_cnta      = 0x1D,
    tca1_cntb      = 0x1E,
    tcb0_capt      = 0x1F,
    tcb0_cnt       = 0x20,
    tcb1_capt      = 0x21,
    tcb1_cnt       = 0x22,
    tcb2_capt      = 0x23,
    tcb2_cnt       = 0x24,
#if defined(TCB3)
    tcb3_capt      = 0x25,
    tcb3_cnt       = 0x26,
#endif
#if defined(TCB3)
    tcb4_capt      = 0x27,
    tcb4_cnt       = 0x28,
#endif
    tcd0_in_a      = 0x29,
    tcd0_in_b      = 0x2A,
    // "Unofficial" user generators. Uses EVOUT, but swaps the output pin using PORTMUX
    evouta_pin_pa7 = 0x8E,
#if defined(Dx_64_PINS)
    evoutb_pin_pb7 = 0x8F,
#endif
#if defined(Dx_48_PINS) || defined(Dx_64_PINS)
    evoutc_pin_pc7 = 0x90,
#endif
    evoutd_pin_pd7 = 0x91,
#if defined(Dx_64_PINS)
    evoutb_pin_pe7 = 0x92,
#endif
#if (defined(Dx_64_PINS))
    evoutb_pin_pg7 = 0x94,
#endif
#endif
#if defined(__AVR_DB__)
    ccl0_event_a   = 0x00,
    ccl0_event_b   = 0x01,
    ccl1_event_a   = 0x02,
    ccl1_event_b   = 0x03,
    ccl2_event_a   = 0x04,
    ccl2_event_b   = 0x05,
    ccl3_event_a   = 0x06,
    ccl3_event_b   = 0x07,
#if defined(LUT4)
    ccl4_event_a   = 0x08,
    ccl4_event_b   = 0x09,
    ccl5_event_a   = 0x0A,
    ccl5_event_b   = 0x0B,
#endif
    adc0_start     = 0x0C,
    evouta_pin_pa2 = 0x0D,
#if defined(Dx_64_PINS) || defined(Dx_48_PINS)
    evoutb_pin_pb2 = 0x0E,
#endif
    evoutc_pin_pc2 = 0x0F,
    evoutd_pin_pd2 = 0x10,
#if defined(Dx_64_PINS) || defined(Dx_48_PINS)
    evoute_pin_pe2 = 0x11,
#endif
#if !defined(Dx_28_PINS)
    evoutf_pin_pf2 = 0x12,
#endif
#if defined(Dx_64_PINS)
    evoutg_pin_pg2 = 0x13,
  #endif
    usart0_irda    = 0x14,
    usart1_irda    = 0x15,
    usart2_irda    = 0x16,
#if defined(USART3)
    usart3_irda    = 0x17,
#endif
#if defined(USART4)
    usart4_irda    = 0x18,
#endif
#if defined(USART5)
    usart5_irda    = 0x19,
#endif
    tca0_cnta      = 0x1A,
    tca0_cntb      = 0x1B,
    tca1_cnta      = 0x1C,
    tca1_cntb      = 0x1D,
    tcb0_capt      = 0x1E,
    tcb0_cnt       = 0x1F,
    tcb1_capt      = 0x20,
    tcb1_cnt       = 0x21,
    tcb2_capt      = 0x22,
    tcb2_cnt       = 0x23,
#if defined(TCB3)
    tcb3_capt      = 0x24,
    tcb3_cnt       = 0x25,
#endif
#if defined(TCB3)
    tcb4_capt      = 0x26,
    tcb4_cnt       = 0x27,
#endif
    tcd0_in_a      = 0x28,
    tcd0_in_b      = 0x29,
    opamp0_enable  = 0x2A,
    opamp0_disable = 0x2B,
    opamp0_dump    = 0x2C,
    opamp0_drive   = 0x2D,
    opamp1_enable  = 0x2E,
    opamp1_disable = 0x2F,
    opamp1_dump    = 0x30,
    opamp1_drive   = 0x31,
#if defined(OPAMP2)
    opamp2_enable  = 0x32,
    opamp2_disable = 0x33,
    opamp2_dump    = 0x34,
    opamp2_drive   = 0x35,
#endif
    // "Unofficial" user generators. Uses EVOUT, but swaps the output pin using PORTMUX
    evouta_pin_pa7 = 0x8D,
#if defined(Dx_64_PINS)
    evoutb_pin_pb7 = 0x8E,
#endif
#if defined(Dx_48_PINS) || defined(Dx_64_PINS)
    evoutc_pin_pc7 = 0x8F,
#endif
    evoutd_pin_pd7 = 0x90,
#if defined(Dx_64_PINS)
    evoutb_pin_pe7 = 0x91,
#endif
#if defined(Dx_64_PINS)
    evoutb_pin_pg7 = 0x93,
#endif
#endif
  };
};
SpenceKonde commented 3 years ago

Yeah, I've seen that style before. Generally speaking, I don't care for it, because it's hard to track how many #if's deep something is - (I couldn't follow something like, say, megaTinyCore's wiring.c, without indentation-levels for #if's, if I couldn't put the cursor before a # and lean on the up arrow until I got to the next # with the same indentation - or like, copy-paste the # and line before it, preceed it with a ^ in regex mode, and find next/prev - that kind of thing....).

However in this situation, with structurally very simple code made hard to read and follow (I definitely agree on that) simply because of the #ifdefs and the indentation that "default spence style) brings with them... Yeah - I have no objection: What I have is absolutely dreadful and I was having problems tracking down a mismatched #ifdef myself (I wound up copy-pasting sections into another file, searching for #(if|endif) and tracking down the odd number). The problem C ran into with the macros is that they ran out of types of brackes/braces within the ASCII character set >.>

I mean, I'd probably indent the second level of #ifdef's one tab-stop or something...

// Generator users
namespace user
{
  enum user_t : uint8_t
  {
#if defined(__AVR_ATmegax08__) || defined(__AVR_ATmegax09__)
    ccl0_event_a   = 0x00,
    ccl0_event_b   = 0x01,
    ccl1_event_a   = 0x02,
    ccl1_event_b   = 0x03,
    ccl2_event_a   = 0x04,
    ccl2_event_b   = 0x05,
    ccl3_event_a   = 0x06,
    ccl3_event_b   = 0x07,
    adc0_start     = 0x08,
    evouta_pin_pa2 = 0x09,
  #if defined(__AVR_ATmegax09__)
    evoutb_pin_pb2 = 0x0A,
  #endif
    evoutc_pin_pc2 = 0x0B,
    evoutd_pin_pd2 = 0x0C,
  #if defined(__AVR_ATmegax09__)
    evoute_pin_pe2 = 0x0D,
  #endif
    evoutf_pin_pf2 = 0x0E,
    usart0_irda    = 0x0F,
    usart1_irda    = 0x10,
    usart2_irda    = 0x11,
  #if defined(USART3)
    usart3_irda    = 0x12,
  #endif
    tca0           = 0x13,
    tcb0           = 0x14,
    tcb1           = 0x15,
    tcb2           = 0x16, 
  #if defined(TCB3)
    tcb3           = 0x17,
  #endif
    // "Unofficial" user generators. Uses EVOUT, but swaps the output pin using PORTMUX
    evouta_pin_pa7 = 0x89,
  #if defined(__AVR_ATmegax09__)
    evoutc_pin_pc7 = 0x8B,
  #endif
    evoutd_pin_pd7 = 0x8C,
#endif

but keeping the ='s lined up is a definite positive for readability and clarity, which is what formatting is for. The fact that it also looks way better too is just gravy.

per1234 commented 3 years ago

I like to put a comment on each #else and #endif to indicate which #if it's intended to match with:

#if defined(__AVR_ATmegax08__) || defined(__AVR_ATmegax09__)
...
#endif  // defined(__AVR_ATmegax08__) || defined(__AVR_ATmegax09__)

As for indentation, I think it can end up being confusing between the code indentation and the preprocessor indentation. What I have seen that seems interesting is indenting after the #:

#if defined(__AVR_ATmegax08__) || defined(__AVR_ATmegax09__)
...
#  if defined(__AVR_ATmegax09__)
   ...
#  endif
...
#endif

The only problem with that is that it's harmful to the searchability of the code because you usually wouldn't think to set up a search to handle arbitrary amounts of space after the # when searching code for a preprocessor directive.

SpenceKonde commented 3 years ago

I'm not really sure how I feel about that in the general case (though I see no problem with the searchability? Just search on # *if and the like... (I get way more milage out of that feature than it deserves :-P - I have at times come damned close to "writing code with regexes"

The commenting each set of macros is all well and good in theory, but ain't nobody got time for that - and I don't think I can write a regex to do that for me :-P ... ... Woah... actually I can....

But I don't think that's worthwhile here, for the most part - there are only a couple of #if's with body longer than a couple of lines. Lemme see what I can make happen without too much work...

SpenceKonde commented 3 years ago

Thank you, by the way!

More porting effort is required to bring Event.h to the tinyAVR platform.

SpenceKonde commented 3 years ago

*hence, I am closing this issue, and will open one over on megaTinyCore.