cnlohr / ch32v003fun

Open source minimal stack for the ch32 line of WCH processors, including the ch32v003, a 10¢ 48 MHz RISC-V Microcontroller - as well as many other chips within the ch32v/x line.
MIT License
913 stars 142 forks source link

Added lib_uart library and example for the CH32V003 #375

Closed ADBeta closed 1 month ago

ADBeta commented 1 month ago

Added my own lib_uart library into extralibs, and added an example program for it in examples

eeucalyptus commented 1 month ago

There are several assumptions in the lib that limit the usability.

In the spirit of this sdk it would be good if those were configurable via funconfig. A nice to have would be compile time format configuration (baudrate, stops, etc), but that's a question of preference I guess

Other than that it's good

cnlohr commented 1 month ago

I agree with all the things @eeucalyptus said.

That's one of the reasons I like making references so users can just copy/paste the code themselves and use it as they see fit, but it would be nice if you were making a reference header, to do those things here. And I def agree that #define's are a good way to shape the behavior of the library.

ADBeta commented 1 month ago

All of the configuration is handled by #defines already, I could move them into a funconfig, but I'm not sure what extra benefit that would serve - if anything personally I find that way more annoying as you have to know WHERE to look to even find your configuration variables. imo it should all be held in the header, but as most of my gripes with ch32v003fun, it's all preference stuff that I really disagree with.

I was toying with the idea of completely removing the non-buffered RX behavior and making it so it always allocates itself a ring buffer for rx, and the size is the only thing configurable. How would either/both of you feel about having it do that, and also having a TX buffer that is always present, you can't turn it off, but you can define its size.

I understand it's kindof frowned upon here to use resources like that, but it seems almost everyone using UART expects it to use RX and TX Buffers, so I feel like it can be excused to some degree to do that

Thanks for the input, the library is still a work-in-progress so I'm trying to round over some of the rough spots :)

eeucalyptus commented 1 month ago

I understand that you purposefully designed the lib as it is and that it covers the constraints that you found as "best way possible". And I even agree to those decusions being the best possible, under the assumptions you made (and the assumptions are good).

However, the reason why this sdk doesn't provide a lot of hardware abstraction is, that the abstraction DOES require assumptions in the first place. When you just have a code snippet that you copy and paste, the effort of customizing it is not zero, but rather negative. You WILL customize it and it will suit your needs to the very last aspect. If it's abstracted and hidden in a HAL, you will do the exact opposite. You will hesitate to even touch the layer in the first place. So you start living with what's available. Yes this is absolutely a philosophical question and 90% of all devs will say "yeah but that way I also don't have to think about it". And its true, it's convenient to not care what's in the box. But at the same time it doesn't cost any brain cell to look inside. If you don't want to change anything or don't know how to do it, then don't! Just leave it as it is. But I think it's safe to say that the vast majority of people WILL understand the code.

That being said, my take is that it's your code, you make the decisions, but others might decide differently. And if you want your code to be as suitable as possible for EVERYONE, I really recommend giving those who want to customize the option to do so. Otherwise they might decide to start from scratch instead, not using your code at all.

Yes there are already defines, but why not use the existing infrastructure ;)