WRTIOT / JennicModuleProgrammer

JenNet-IP-Border-Router-JennicModuleProgrammer
6 stars 10 forks source link

Add support for JN5169 #1

Open Jerome-PS opened 6 years ago

Jerome-PS commented 6 years ago

Adding support for JN5169. Adding support for macOS.

qinfengling commented 6 years ago

Thanks very much for your cool PR. Could you split it to 2 commits ? I will merge the macOS first.

And also please keep commit with the code style before.

Thanks.

Jerome-PS commented 6 years ago

Hi,

I was starting to do as you asked, but I saw you were at it... So do you want me to do anything?

PS: I agree that having mixed coding styles is very ugly, but don’t complain to me if a bug appears with the original coding style ;oP More seriously it is mostly a question of taste EXCEPT when it comes to if/for/while/etc blocks that are not surrounded by curly brackets, this has bitten me so many times! (Plus all the times spending minutes to try and match brackets after copy/paste...)

PPS: could the code be changed to GPL?

PPPS: if you are NXP, do you have more docs about the chip? The doc for the boot loader is not up to date and come on, just because you provide a lib doesn’t mean coders do not need the chip’s register description!

Cheers, Jérôme.

Le 17 janv. 2018 à 07:45, qinfengling notifications@github.com a écrit :

Thanks very much for your cool PR. Could you split it to 2 commits ? I will merge the macOS first.

And also please keep commit with the code style before.

Thanks.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

Jerome-PS commented 6 years ago

Well, this is not a question of device, it is a question of integer math. The base frequency is 1000000Hz, so you can’t have 115200, only 111111... or 125000. The closest is 111111 (-3.5% error). 125000 is out of RS-232 specs with (8.5% error).

Basically adding half the value you divide, adds 0.5 to the value before truncating. This gives the correct (as indicated in the JN-AN-1003 boot loader data sheet) 9 for 115200 and 26 for 38400.

Cheers, Jérôme.

Le 17 janv. 2018 à 07:43, qinfengling notifications@github.com a écrit :

@qinfengling commented on this pull request.

In Source/JN51xx_BootLoader.c:

@@ -664,9 +742,11 @@ teStatus BL_eSetBaudrate(int iUartFd, uint32_t u32Baudrate) uint8_t au8Buffer[6]; uint32_t u32Divisor;

  • // Divide 1MHz clock bu baudrate to get the divisor
  • u32Divisor = 1000000 / u32Baudrate;
  • // Divide 1MHz clock bu baudrate to get the divisor, round to closest
  • u32Divisor = (1000000+u32Baudrate/2) / u32Baudrate; Is this correct settings for other devices?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

qinfengling commented 6 years ago
  1. About the code style yes, the original code was written by NXP, we try our best to make sure the code is easy to read. I just push the original code here for developer, when I use the JN516x, I found that the share is hard, so I push the archive here.

  2. could the code be changed to GPL? I will not change that. NXP release the code as opensource. it will changed by them.

  3. if you are NXP, do you have more docs about the chip? ... Sorry, I am not work for them. so you may try to search the docs through their website or ask it from FAE.

Keep going.

Fengling