Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed sending message from PC / Fix for ENABLE_MESSENGER_UART #120

Closed
wants to merge 5 commits into from

Conversation

cb1986ster
Copy link

Based on MCFW.

You can connect via serial port for sending and receiving messages. For sending just send SMS:Your message. Any time radio receive message it will be send via serial prefixed like SMS>Recived message. I am also planing to make some pythonlib to connect to my rpi and/or make simple web app to support this.

app/uart.c Outdated
#ifdef ENABLE_MESSENGER
#ifdef ENABLE_MESSENGER_UART

if (UART_DMA_Buffer[gUART_WriteIndex] == 'S' && UART_DMA_Buffer[gUART_WriteIndex + 1] == 'M' && UART_DMA_Buffer[gUART_WriteIndex + 2] == 'S' && UART_DMA_Buffer[gUART_WriteIndex + 3] == ':')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use memcmp() here for cleaner code?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was also wondering if we need to require data to start with SMS, instead of sending any serial data that comes in?
So it could be a bit more universal. Are there any implications of this?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh nevermind I guess reading from CHIRP would then trigger message send. Perhaps in the future it should be limited to only when messenger screen is active.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe if (strncmp(((char*)UART_DMA_Buffer) + gUART_WriteIndex, "SMS:",4) == 0) ?

@@ -894,5 +886,15 @@ void MSG_ClearPacketBuffer()
memset(dataPacket.serializedArray, 0, sizeof(dataPacket.serializedArray));
}

void MSG_Send(const char *cMessage){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this refactor. Good job!

Copy link
Owner

@kamilsss655 kamilsss655 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Thank you for submitting PR. I wanted to add this but was focused on other functionalities. This certainly helps.

I haven't tested it yet, but left a few questions for you.

@@ -102,3 +103,16 @@ void UART_LogSend(const void *pBuffer, uint32_t Size)
UART_Send(pBuffer, Size);
}
}

void UART_printf(const char *str, ...)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reuse LogUart() from debugging.h ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not for debug, but for send/recv via serial port. I think it should not be mixed.

@kamilsss655
Copy link
Owner

Seems like the UART receive portion is a bit buggy as it loses some data, ie:

---- Sent utf8 encoded message: "SMS: This is a test message" ----
SMS> This is a test mess
SVC<RCPT

@cb1986ster
Copy link
Author

Seems like the UART receive portion is a bit buggy as it loses some data, ie:

---- Sent utf8 encoded message: "SMS: This is a test message" ----
SMS> This is a test mess
SVC<RCPT

I saw this once during testing but I thought that it is not on serial print, because at the radio display it also was not complete. UART_printf routine has limit of 256 and default PAYLOAD_LENGTH is 30. vsnprintf?

Bartosz Celmer added 3 commits February 2, 2024 00:02
Send:
LED1 - to turn on LED
LED2 - to turn on LED Blink 
LED3 - to turn on LED SOS
LED3 - to turn off LED
@cb1986ster
Copy link
Author

Also adding new feature: ENABLE_MESSENGER_LED - controlling radio led remotely

Send:
LED1 - to turn on LED
LED2 - to turn on LED Blink
LED3 - to turn on LED SOS
LED3 - to turn off LED

My assumption is to use this along with encryption. This could possibly be enabled in the menu, instead of at compilation level.

@kamilsss655
Copy link
Owner

Also adding new feature: ENABLE_MESSENGER_LED - controlling radio led remotely

Send: LED1 - to turn on LED LED2 - to turn on LED Blink LED3 - to turn on LED SOS LED3 - to turn off LED

My assumption is to use this along with encryption. This could possibly be enabled in the menu, instead of at compilation level.

I am partial on this feature, however I see how it might be useful for search & rescue use cases.
I will not take it in this PR though, as there are bunch of things mixed in single PR (including changes to FM radio). For the future please submit PR's with single functionality, if functionality is something completely new please use new Makefile flag. You can first create an enhancement issue so we can discuss the details there.

Couple of suggestions if you decide to submit a new PR with LED control:

  • instead of checking for message content create new COMMAND_PACKET
  • create a new Command enum:
    • LED_ON
    • LED_SOS
    • etc
  • and the corresponding char array of Command values:
    • CMD:LEDON
    • CMD:LEDSOS
    • etc
  • this whole functionality should be under separate makefile flag (as you submitted, Makefile flag option was missing)


ST7565_BlitFullScreen();
}

void UI_DisplayFM(void)
{
UI_DisplayFM_text("");
Copy link
Owner

@kamilsss655 kamilsss655 Feb 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whereas I like this improvement, the implementation is not slim with the wrapper function approach. And considering we're at 99% mark of space utilization it is quite important.

It could be rewritten to use a global boolean (which probably is already defined, but not used) to indicate scan in progress and then the ui function can determine whether or not display SCN.

@kamilsss655 kamilsss655 closed this Feb 2, 2024
VA7FGT pushed a commit to VA7FGT/uv-k5-firmware-custom that referenced this pull request Apr 16, 2024
…ease, long press EXIT exits monitor mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants