n-wach / camino

A library for controlling an Arduino from Python over Serial
MIT License
16 stars 3 forks source link

Adding modified files for the Arduino Nano Every #17

Closed OwlvilleWol closed 4 months ago

OwlvilleWol commented 4 months ago

Hi Nathan,

I don't think this actually should be merged. I just wanted to let you know it's there, in case you're or anyone else is interested. Plus I wanted to take the opportunity to commend you on the nice library. I've been looking for something similar for my project and none of the other popular ones (e.g. Serial Transfer) match the simple elegance of Camino.

Anyway I needed it to work on the Arduino Nano Every, which uses the ATMega4809. It has a different register structure. I tried to insert it elegantly into the #define decision tree, but changes go deeper than that. E.g. the serial data register is different for RX and TX. So I just ended up removing all of that and replacing most of the init code with effectively what's in the native Serial.begin().

Doesn't look too good, but functions great. Thanks again for your work and for sharing.

n-wach commented 4 months ago

This is awesome, thanks for sending it over! Glad you found camino to be helpful :)

I'll keep this as a PR for now, hopefully other people can use it. It would be fantastic to get it merged with the normal library, but you're right my naive #define tree is not designed for extensibility like this...

n-wach commented 4 months ago

I'd love to improve the config UX and add support for more architectures. Ideally:

If you're willing to test some changes on your end with your board, I might take a crack at these improvements and try to merge this PR

OwlvilleWol commented 4 months ago

I'll test the heck out of it if you implement any of that. I dug up the basement, but I only have Nano Everys and one Due, but that's ARM based. I'll check at work if I can find any newer AVR units that have a different register map.

n-wach commented 4 months ago

Release 1.7.0 refactored the way camino supports architectures. Please take a look at src/arch.h--I hope it's abstract enough

If you could modify this PR (or create another) to add ATMega4809 support, I'll be happy to merge :)

OwlvilleWol commented 4 months ago

That's awesome. It looks straightforward enough.

OwlvilleWol commented 4 months ago

Allrighty... it's 4am here, but I made it work on the Due, which is SAM3 based. The Every Nano wasn't difficult but this gave me a challenge. On the Due It uses the Serial library as opposed to low level, but I guess that's not that big of a deal considering it's a much faster mcu. Miraculously, with a bit of creativity, it fits in your #define tree. I need to clean it up a lot, but I'll upload it after.

n-wach commented 4 months ago

That’s awesome! This will be a great addition the library, thanks for figuring it out :)

OwlvilleWol commented 4 months ago

I'm not going to lie, I had doubts when I saw your .h only implementations. Turns out I'm the stupid, not your solution. Maaan, Arduino is not making it easy. Anyway after another couple of hours down the drain I think I have a reasonably ... wonky in a different way? ... solution. I'll try to submit it tomorrow and you can decide if you want to blow up the whole thing again.

On a different note. In the python module, line 134. Is that "list" on purpose? Am I missing something? The call itself is marked with out=bytes, so I'd expect a bytes object, but this way I have to put that in a list. (I can open an issue if you want me to.)

n-wach commented 4 months ago

Yeah, not a fan of using only .h, but pretty sure it's required by Arduino IDE if I want to let people #define prior to #include...

In camino.Callable.call(*args), args is the list of unnamed arguments passed to the function. The args need to be translated to an input list of bytes, which are then sent to the Arduino. The code (starting on line 126) is kinda hacky:

Some examples:

The out=bytes is just declaring the named argument out, and setting the default value to bytes. out controls the output format, and is used starting on line 146

You're right I should add an input case for bytes (although wrapping the bytes in a list does work: s = b'abc'; add(list(s)))