hideakitai / ArtNet

Art-Net Sender/Receiver for Arduino (Ethernet, WiFi)
MIT License
257 stars 52 forks source link

Create ArtnetEtherENC.h #79

Closed MrFrangipane closed 10 months ago

MrFrangipane commented 10 months ago

Hello

Thank you for this awesome library.

Is it possible to make possible the use of the following library ?

Allows to use https://github.com/JAndrassy/EthernetENC

For now, I modifiy files manually, it would be awesome to have this embedded in the lib, if It helps someone else

All the best, Val

MrFrangipane commented 10 months ago

Hi !

Thanks for your response !

I'll look into it :)

Val

MrFrangipane commented 10 months ago

Hi,

I tried to add what you requested :)

The wiring diagram was taken from https://github.com/Juddling/pi-pico-enc28j60 and is reportedly made by https://github.com/tobiasvogel

I'm not familiar with workflows I copied/pasted previous sections

Hope this helps, All the best !

MrFrangipane commented 10 months ago

Also,

It's probably not the place to ask you about this (sorry), I see here than only four callbacks are allowed https://github.com/hideakitai/ArtNet#subscribing-callbacks-with-net-sub-net-and-universe-as-you-like, why is it so ?

Thank you again, Val

hideakitai commented 10 months ago

About callbacks, the limitation has been removed, but README is not fixed…

https://github.com/hideakitai/ArtNet/issues/59

MrFrangipane commented 10 months ago

About callbacks, the limitation has been removed, but README is not fixed…

59

Oh OK i was surprised to see it working, do you want me to remove this section from the readme as well ?

hideakitai commented 10 months ago

Oh OK i was surprised to see it working, do you want me to remove this section from the readme as well ?

It’s great if you can! Thank you!

MrFrangipane commented 10 months ago

I see some builds have failed

Should we remove those boards from the build and list supported boards in the docs ?

   /home/runner/Arduino/libraries/ArtNet/Artnet/ArtPollReply.h:68:15: warning: 'my_mac' is used uninitialized in this function [-Wuninitialized]
           memcpy(r.mac, my_mac, 6);
           ~~~~~~^~~~~~~~~~~~~~~~~~
MrFrangipane commented 10 months ago

Oh OK i was surprised to see it working, do you want me to remove this section from the readme as well ?

It’s great if you can! Thank you!

It's done, I copied the explanation you gave in #59

https://github.com/hideakitai/ArtNet/blob/b4df2244faa6df0d9837bcf8c3ecfa981039d0bf/README.md#subscribing-callbacks-with-net-sub-net-and-universe-as-you-like

Although I'm not sure what is meant by "making public 4 of them"

hideakitai commented 10 months ago

Although I'm not sure what is meant by "making public 4 of them"

PollReply has the information of four input/output ports. So current implementation is, you can subscribe many universes but four of them are informed by PollReply. (But this implementation is not preferred, so I should fix it https://art-net.org.uk/how-it-works/discovery-packets/artpollreply/)

hideakitai commented 10 months ago

@MrFrangipane Maybe the suggestions above can fix workflows. Could you try to implement them?

MrFrangipane commented 10 months ago

Although I'm not sure what is meant by "making public 4 of them"

PollReply has the information of four input/output ports. So current implementation is, you can subscribe many universes but four of them are informed by PollReply. (But this implementation is not preferred, so I should fix it https://art-net.org.uk/how-it-works/discovery-packets/artpollreply/)

Thank you ! I get it now :)

MrFrangipane commented 10 months ago

@MrFrangipane Maybe the suggestions above can fix workflows. Could you try to implement them?

Sure, changes have been made :)

I realized too late i could directly commit your suggestions, I'll know for next time

hideakitai commented 10 months ago

Ah, maybe this is a bug of EthernetENC…

MrFrangipane commented 10 months ago

Ah, maybe this is a bug of EthernetENC…

It says on EthernetENC's readme

For ESP32 it is better than using a MAC layer module like the enc28j60 to use a PHY layer LAN module like LAN8720 supported by the ESP32 MAC peripheral and the Arduino boards support package.

Should we drop ESP32 from those tests and state to not use ESP32 with EthernetENC in the readme ?

hideakitai commented 10 months ago

Please comment out esp32, s3, and c3 from workflow. But it is NOT unavailable for esp32 series, I will send pull request to EtherENC :)

hideakitai commented 10 months ago

state to not use ESP32 with EthernetENC in the readme ?

Could you add a same note as README of EtherENC?

For ESP32 it is better than using a MAC layer module like the enc28j60 to use a PHY layer LAN module like LAN8720 supported by the ESP32 MAC peripheral and the Arduino boards support package.

hideakitai commented 10 months ago

Created a PR on EthernetENC https://github.com/JAndrassy/EthernetENC/pull/58

-> merged

hideakitai commented 10 months ago

If you finish working on this PR, please mention me!

MrFrangipane commented 10 months ago

Hi @hideakitai

Thank you for your help again !

I realized only 2 examples were compiled, so I added them to the workflow

sketch-paths: |
  - examples/EthernetENC/receive_fastled
  - examples/EthernetENC/receiver
  - examples/EthernetENC/send_receive
  - examples/EthernetENC/sender

This revealed that FastLED is not supporting mkrvidor4000 boards. What do you prefer ?

I checked the other workflows and they seem to correspond to the 2nd option. The current version of the workflow is as follows

sketch-paths: |
  - examples/EthernetENC/receiver
  - examples/EthernetENC/send_receive
  - examples/EthernetENC/sender
# disabled due to FastLED not supporting mkrvidor4000
# - examples/EthernetENC/receive_fastled

If this is OK for you, merging should be OK 🥳

hideakitai commented 10 months ago

Thank you! Yeah, mkrvidor is not supported. Hmm.. I prefer

remove the board from tests and leave fastled enabled for the others

MrFrangipane commented 10 months ago

Thank you! Yeah, mkrvidor is not supported. Hmm.. I prefer

remove the board from tests and leave fastled enabled for the others

@hideakitai board has been disabled and FastLED reinstated

All tests seems to have passed :)

hideakitai commented 10 months ago

All checks passed, thank you! I'll review the code again :)

MrFrangipane commented 10 months ago

There is one typo, but LGTM

Suggested fix was commited :)

hideakitai commented 10 months ago

Thank you for your contribution!

hideakitai commented 10 months ago

Please close https://github.com/hideakitai/ArtNet/issues/80 :) Thanks again!

MrFrangipane commented 10 months ago

Awesome !

Thank you for all those wonderful libraries 🙏