nanovna-v2 / NanoVNA2-firmware

Firmware for NanoVNA V2
https://nanorfe.com/nanovna-v2.html
222 stars 87 forks source link

Fix frequency_string_short and its usage #20

Closed wutje closed 4 years ago

wutje commented 4 years ago

When showing multiple markers there where two issues:

douardda commented 4 years ago

why changing the frequency_string_short() signature to make it return an int? Is this returned values used / planned to be used somewhere ?

wutje commented 4 years ago

Yes, I am working on code that uses this same function to print the current frequency on recall buttons. That is how I found this bug. Printing frequency with suffix can be hard and I rather reuse code then invent it again. But for my code to work I need the result of the snprintf.

DiSlord commented 4 years ago

See NanoVNA chprintf.c file for print frequence need only use "%q" and also support any formatting For print value vs suffix like 0.0001 use "%F" and get 100u Possible format "%+5.2F" for example and other printf formats

douardda commented 4 years ago

Yes, I am working on code that uses this same function to print the current frequency on recall buttons. That is how I found this bug. Printing frequency with suffix can be hard and I rather reuse code then invent it again. But for my code to work I need the result of the snprintf.

Ok then this should be explained in the commit message IMHO (or be part of a dedicated commit).

wutje commented 4 years ago

@douardda I changed the code. It no longer returns the amount of characters printed. It is better take small steps. My other code is obsolete now due to @DiSlord great work on the GUI. @DiSlord https://github.com/gabriel-tenma-white/mculib/blob/master/printf.cpp does not have %q?

wutje commented 4 years ago

Can this please be merged? It came up again as #40 . Measuring a 2400 (Wifi, Bluetooth etc) filter is nearly impossible. Is there anything I can or need to do so this can be merged?

gabriel-tenma-white commented 4 years ago

Sorry, thought this was already included in another PR. It's merged now.

wutje commented 4 years ago

Great! Thank you so much for the quick response.

DG9BFC commented 4 years ago

when can we expect a new release ?? i am no programer (just a user) so i have no idea how to compile a fw ... (i can upload a new one ... thats easy) ... i own TWO saa2 ... one with small and one with big display (4 inch and n sockets) so best would be compile both versions and release them (like we have seen some months ago) THANK YOU dg9bfc sigi

douardda commented 4 years ago

when can we expect a new release ?? i am no programer (just a user) so i have no idea how to compile a fw ... (i can upload a new one ... thats easy) ... i own TWO saa2 ... one with small and one with big display (4 inch and n sockets) so best would be compile both versions and release them (like we have seen some months ago) THANK YOU dg9bfc sigi

indeed, I've opened an issue for this #41

douardda commented 4 years ago

@DG9BFC I've compiled the current master (rev e017aab5) and put them here https://github.com/douardda/NanoVNA-V2-firmware/releases/tag/20200905

I've only tested the 2.8" (ili9341) version since I do not have a 4" display yet (should arrive in a few days hopefully).

Hope this helps.

DG9BFC commented 4 years ago

tested on small and big screen ... seems to work fine (both fw versions tested!! ... small and big screen!!) so i think you can close issue 41 :-) thank you

lzqgdt commented 4 years ago

tested on big screen ,no problem .