ssilverman/TeensyDMX

Teensy 4.1 Native Ethernet conflict? Link light callbacks.

s4CD opened this issue · 35 comments

s4CD commented

Moved to its own issue as requested:

I'm also using NativeEthernet on this board and teensyDMX seems to kill the linkStatus functionality due to link_callback not being received, yet works fine without this librariy initialized. The linked libraries are to utilize the native ethernet on the Teensy4.1 production boards.

Using the following libs:
https://github.com/vjmuzik/NativeEthernet
https://github.com/vjmuzik/FNET

Off the top of my head, maybe there's some conflicts with the interrupts or timers. I'll have a look when I have a chance. Thanks for reporting this.

@s4CD, to clarify, when you say "works fine without this library initialized", do you mean that NativeEthernet works fine if TeensyDMX is not initialized?

@s4CD, second question: Can you paste some source code here that shows the issue? I'm going to engage @vjmuzik once I understand the specific problem, but I'd like to be armed with more information. Thanks!

From giving this a quick glance I would say that your PeriodicTimer conflicts with the builtin Teensy IntervalTimer since they are both accessing the PITs so one would cancel out the other.

s4CD commented

I apologize, fell asleep on you! Yes.. using NativeEthernet alone the link light callbacks work. As soon as I add teensyDMX it kills it. Followed the functionality all the way through NativeEthernet and all seems ok up until the link_callback function within NativeEthernet, and it will not fire that callback when teensyDMX is used. Whatever state is read on startup stays forever (if Ethernet connected when powered on, link light will light and stay regardless of cable being plugged in. If unplugged on startup light will never light)

I can try to put something together later today. All that is needed is to start Native Ethernet and teensyDMX.. you will need the cap and jack connected though to test, so I can do so if you do not have those components.

I'll make an effort to see what I can do to fix this.

Some background: Teensy Core's IntervalTimer uses the Arduno "everything's a global variable" approach, and it's difficult/messy to use it to implement callbacks, where the callback knows about who called it. There's not even any state variable you can pass in to help implement this. That's why I created my own PeriodicTimer. It does similar things, but implements a way to pass information when starting a timer. (See: PaulStoffregen/cores#291)

@vjmuzik Can you point me to where NativeEthernet or FNET use IntervalTimer? A quick GitHub search shows no results in either repo. I want to see its exact usage and if I can work around it.

@s4CD I'm curious, can you change the initialization order of TeensyDMX and NativeEthernet and see if that changes anything? i.e. if you initialized TeensyDMX first, then initialize NativeEthernet first, etc. I don't expect it to work, I'm just curious if any sort of behaviour changes at all.

Some options for me to try:

  1. See if I can work with Paul's repo to get that "pass state in" change into IntervalTimer so that I can use it instead, and thus not conflict with other libraries' usage. (Update: See PR PaulStoffregen/cores#490)
  2. See if there's a way I can improve my PeriodicTimer so that it can better detect other users of those PIT timers.
  3. Go back to figuring out some messy way I can use the current IntervalTimer to fit my callback and state-passing needs.

Yeah it’s not immediately obvious where I’m using it since I only make one call to it, all it does is create an interrupt timer that polls the FNET stack by a default time of 1ms.

I wish luni64’s TeensyTimerTool was a part of Teensyduino since it can auto/manual assign to more than just the PITs.

https://github.com/vjmuzik/NativeEthernet/blob/a03afd169b15a5441d474231426929ee434dc270/src/NativeEthernet.h#L84
https://github.com/vjmuzik/NativeEthernet/blob/a03afd169b15a5441d474231426929ee434dc270/src/NativeEthernet.cpp#L31
https://github.com/vjmuzik/NativeEthernet/blob/a03afd169b15a5441d474231426929ee434dc270/src/NativeEthernet.cpp#L140

s4CD commented

I tried reversing the order, made no difference. I also do not need to initialize teensyDMX (I am using the Sender function) for it to break. Just having "#include <TeensyDMX.h>" will do it. Not sure if that helps anything or not..

I haven’t forgotten about this. I still intend to address the potential timer issue and then test (with some help), once I have some time to get into it.

@s4CD thanks for your patience.

s4CD commented

All good, I've been tied up in another project for a bit. I'm definitely able to do any testing you need for transmitting data. I have access to a lot of different types of DMX fixtures, just nothing with RDM.

@s4CD I made a new branch which has the ability to switch to using Teensyduino's IntervalTimer class instead of my custom one: https://github.com/ssilverman/TeensyDMX/tree/use-interval-timer
If the TEENSYDMX_USE_INTERVAL_TIMER compiler define is set then it will switch to using IntervalTimer. You can see an example of this in the https://github.com/ssilverman/TeensyDMX/blob/use-interval-timer/platformio.ini file.

Can you try this with your Ethernet library integration and see if it works for you?

The ramifications are that the transmit BREAK timings will be less accurate.

@s4CD pinging you again if you’d still like to test this.

s4CD commented

Absolutely! Sorry for the delay, been a crazy week but I should be able to look at it this weekend and let you know what I can find.

No worries. Thank you for helping and thanks for your patience with my delay. I figured you might have missed the first notification, so I pinged again. :)

@s4CD could you provide me with the software you used to test? A cut & paste should be fine. Even though I don’t have the hardware, my theory is that I’d be able to observe the issue.

s4CD commented

Sorry again for the delays.. we have hit my busy time and my schedule changes on me unexpectedly. I did try it out but was getting strobing on the lights rather than the correct sequence. This occurred with or without the flag set, so I need to roll back and verify my hardware is still set up right. I needed to test something else with this board and move some stuff. I will update shortly.

As for the sketch, I will try to upload that soon, but there is some code from a coworker I need to clear before posting. I'm going to try to set up a minimal sketch to test with. Basically just using native Ethernet to get on the network works as expected. (No code needed beyond network connection) Including TeensyDMX library (no actual code to use TeensyDMX is necessary) and link light stops working. The link light will retain the state it was on power up. The current lib did not fix link light issue with interval defined or not, and lights did not work with or without, which is why I want to confirm hardware before saying the new code has issues.

s4CD commented

It was definitely a hardware issue. DMX output seems to work fine with the use interval flag set or not. Link light remains unaffected by physical link status. If unplugged on power up there is never a light, if plugged in on power up the light is always there. Removing TeensyDMX include makes link light work as expected.

Thanks for the info. Was TEENSYDMX_USE_INTERVAL_TIMER set before the TeensyDMX include? And this was for sure the code from that branch?

Sounds like the bug is still there.

I think maybe I should get some Ethernet hardware. What do you recommend getting to try this out?

s4CD commented

I will put together a simplified sketch tonight. In hindsight I may have added the include after.. I had to run to the office but will verify that tonight as well.

As for hardware, I am using a custom PCBA but the ethernet section is the same as the Teensy Ethernet module here: https://www.pjrc.com/store/ethernet_kit.html

The define must come before the include.

#define TEENSYDMX_USE_INTERVAL_TIMER
#include <TeensyDMX.h>

Thinking about it more, I’m wondering, with the way I’ve done this, if it needs to also be passed to the compiler. Which platform are you using to build and run this code? The Arduino/Teensyduino IDE, PlatformIO, or other?

@s4CD okay, I just pushed a change to the use-interval-timer branch that forces that define directly in the header. That should make testing easier. If you’re using the right branch, you’ll see #define TEENSYDMX_USE_INTERVAL_TIMER near the top of TeensyDMX.h.

No more putting in the define yourself. :)

s4CD commented

yes.. momentary lapse of reason.. lol! Sorry about that. I will test when I get home. Should be in the next hour or two.

Awesome!

Thanks for that link; I ordered an Ethernet adapter.

s4CD commented

No problem! One thing to note that I did not think of before.. That PCB has no link light connection. The 6p ribbon has T+, T-, R+, R-, GND, ACT. To enable link light you will need to solder to the pins on the bottom of the Ethernet port. If you look at the footprint of the port it has 12 pins plus two big holes for alignment and two slightly smaller holes for the casing. Out of the 12 actual pins 8 are in two rows of 4, which are the ethernet jack pins. The remaining 4 are split into two groups of two closest to the actual port on the jack. One group of two has traces connected on the PCB, the other group does not. The pair without traces is the link light. One side needs ground and the other connects to a GPIO output through a 330OHM resistor.
TeensyEthernet_LinkLight

I’m half-following. What do I need to connect and to where?

Update: Oh, I see. The red arrows.

Update 2: Which GPIO? Also, it sounds like I can just query the GPIO on change or at regular intervals or something instead of soldering this?

Update 3: Instead of using the hardware, since it’s just on a GPIO anyway, why do I need the actual hardware?

s4CD commented
  1. As you said, arrows should clarify.
  2. I used 33. See attached file. Can be any you like.
  3. The issue is not making an LED light from GPIO, the issue is receiving the callback that triggers the light to change. I don't know of a way to trigger that without plugging/unplugging the cable..

I have attached a basic sketch. It is the most basic initialization of ethernet and a basic implementation of TeensyDMX. The setup puts link light on GPIO33, but you can move it to any you like. For ease of testing the serial monitor also will display current link state each loop so you can view that without looking at the light or if you dont want to rig up the physical light. As-is all TeensyDMX is commented out with an open/close indicator:

//*****Remove to test without DMX---------------------------------------------------------- //#define TEENSYDMX_USE_INTERVAL_TIMER //#include <TeensyDMX.h> //*****------------------------------------------------------------------------------------

While in this state link light works perfect. Uncomment all of the DMX related lines (4 groups: Lines 3-4, 25, 34, and 89) and it will output DMX on Ser8 (you can change to use any serial you like) which just slowly ramps up red, then green, then blue, then back again. While this is running link light no longer works.

Originally it stopped if you only uncomment the include. Your latest update with the TEENSYDMX_USE_INTERVAL_TIMER defined allows the link light to work after include is uncommented (line 3-4), and still works after declaration of Sender (line 25), but stops as soon as you uncomment the begin() function call (line 34).

So the last update DID actually make an improvement, but still has a conflict after begin() is called. I also have a delay at the end of the loop. The sketch controls the light smoothly WITHOUT the define of TEENSYDMX_USE_INTERVAL_TIMER with no delay, but as soon as that is defined the light strobes randomly unless the delay is added.. I am starting to fear I am leading you to break your lib in order to save a function of another...

TeensyETHtesting.zip

s4CD commented

Well now it seems to work without the delay.. I must have had something wonky in the function to ramp the lights..

I don’t mind investigating this because other libraries may conflict with how I use the periodic timers. The worst that happens is the send BREAK timing is less accurate, but the user will have a choice.

Thanks for trying this out and helping me fix it. I’ll be able to test alongside you with my own Ethernet setup in due time.

To clarify, what do you mean by conflict? I’m hearing two things, tell me if I’m getting them right:

  1. The DMX send is wonky when TeensyDMX is used concurrently with Ethernet, and
  2. The link light works, but only when you #define TEENSYDMX_USE_INTERVAL_TIMER.

Do I have those two facts right?

s4CD commented

Sorry, that was unclear..

  1. my DMX output appeared to get very choppy and strobing when I used the interval define, but I think something in the way I was doing my color values in the loop was off. It seemed to only work well if I added a delay, but after messing with it a bit it now works without a delay and without looking choppy. Nothing to do with the link light yet, just DMX output.

  2. Link light still is not working, but it is getting closer. Originally just line 4 killed link light function, the include. While using the interval define the include does not kill it. Link light works until the begin() function is called now, at that point link light no longer works. DMX works fine the entire time. It is actually still going from when tested yesterday just looping through colors.

So I've been thinking a lot about how to fix this problem. I've now observed the issue for myself (thank you, @s4CD, for getting me started) and have a complete fix. I'm weighing several options for the release, maybe y'all have some opinion. But first, the cause:

IntervalTimer is used by a number of libraries, and if I wish to use a PIT timer and be compatible with other libraries that use it (such as NativeEthernet (thanks, @vjmuzik!)), I must also use this class. The problem is that once other libraries start getting involved, it's possible for IntervalTimer's begin function to return false, which then means that pin timing becomes hard to do. Let me explain.

Say I want to toggle a pin for a specific amount of time. I'd set the pin, start the timer for a set amount of time, and then, when the timer expires, I unset the pin. I can account for the small time difference between setting the pin and starting the timer because it's my code. More on that shortly. Because begin may return false, the only option is to set the pin after begin returns, otherwise, it's erroneously set and then unset (thus toggling for an incorrect amount of time) for that false return. However, once begin returns, a timer has already started. Because this is not my code and since this may change between releases, technically an unknown amount of time has passed between starting the timer and setting the pin. I'd have to track all the releases for my adjustment variable.

Therefore, we need a way to call some sort of "start" function (that, here, sets the pin) only if begin would return true, but just before the timer is started. This is one of the reasons I made my own PeriodicTimer class; it has this ability. But it's not compatible with uses of IntervalTimer because they both separately keep track of which timers are active and none of that state is accessible from IntervalTimer, so I can't really use it.

I've currently defined a TEENSYDMX_USE_INTERVAL_TIMER compiler variable that uses PeriodicTimer when unset, and IntervalTimer when set, but for this second case, we can't make use of variable BREAK and MAB timings in the protocol when transmitting. i.e. it uses default fixed values (or I can choose to make it a little less accurate). Now, this may not be a problem for most users, but users that want it need PeriodicTimer.

So here are the options, as I see them:

  1. Don't set TEENSYDMX_USE_INTERVAL_TIMER. (Or set TEENSYDMX_USE_PERIODIC_TIMER.) To be compatible with other libraries, a user would have to set this value (or unset the other value) in the TeensyDMX.h header file (or pass it as an extra compiler option).
  2. Set TEENSYDMX_USE_INTERVAL_TIMER. (Or don't set TEENSYDMX_USE_PERIODIC_TIMER.) To use custom transmit BREAK and MAB timings, this would have to be unset (or set, for the other variable) inside TeensyDMX.h.
  3. Have an API that can toggle between the two modes, but this might require slightly more memory or program space, and I'd prefer to keep it smaller (eg. for Teensy LC).
  4. Convince Arduino to make it easier to pass in compiler defines on build. Probably not happening in the near future.
  5. Convince everyone else to use my PeriodicTimer. This isn't feasible.
  6. Update IntervalTimer with a pull request. This may not be accepted, and it may take a long while.

The best options, I think, are nos. 1 & 2. The correct choice, then, might be the option that requires the lease number of users to modify the header. I'm thinking that custom BREAK and MAB timings are a more advanced use case, and so maybe option 2 is the best for now. The flip side of these two options is to instead define TEENSYDMX_USE_PERIODIC_TIMER in the opposite sense of the other variable, and if those custom timings were required, the variable would have to be added (or passed as an extra compiler option). So maybe it's option 2, but with an unset TEENSYDMX_USE_PERIODIC_TIMER?

@s4CD the reason defining that variable before including <TeensyDMX.h> in the program wasn't working is that there's more than one source file that uses this header and they're all compiled separately. This means we need to either set an extra compiler option or add it to the header directly. That's what I'm proposing now.

Thoughts? Comments? Suggestions?

I believe this is solved with the latest release, v4.1.0-beta.2.

Note: see also my https://github.com/ssilverman/QNEthernet Ethernet library, which doesn’t use timers.