![]() |
![]() |
#1 |
Registered User
Join Date: Jan 2020
Location: Poland
Posts: 182
|
[Commodity] Commodity that monitors status - code review request
Hi,
Could you please review this commodity code. I want it to execute some commands in given time interval. I checked it in the system with cpu idle monitor and it looked fine - cpu was at minimum. I just want to make sure that the timer and waiting command are done ok, so the application wont lag the system. ps. its not yet finished (and not polished) - but I would like to know if there is some kind of error in this on this level? the final code will run simple tcp connection routine to check the internet status and will save it in ENV variable. Thanks in advance.. ![]() ![]() Code:
#include <proto/exec.h> #include <proto/dos.h> #include <clib/alib_protos.h> #include <clib/commodities_protos.h> #include <libraries/commodities.h> // Libs struct Library* CxBase = NULL; struct Library* SocketBase = NULL; // Commodity globals struct NewBroker cx_newbroker = { NB_VERSION, // nb_Version - Version of the NewBroker structure (STRPTR)"msInternetStatus", // nb_Name - Name CX uses to identify this commodity (STRPTR)"msInternetStatus v0.1", // nb_Title - Title of commodity that appears in CXExchange (STRPTR)"This Commodity executes any command or script in regular time intervals.", // nb_Descr - Description of the commodity 0, // nb_Unique - Tells CX not to launch new commodity with same name 0, // nb_Flags - Tells CX if this commodity has a window 0, // nb_Pri - This commodity's priority 0, // nb_Port - MsgPort CX talks to 0 // nb_ReservedChannel - reserved for later use }; struct MsgPort *cx_broker_message_port; CxObj *cx_broker; ULONG cx_sign = 0; // --- timer globals --- struct MsgPort* timer_message_port; struct timerequest* timer_io; struct Message *timer_message; struct IOExtSer *reply; ULONG timer_sign = 0; int numer = 0; int main(void) { timer_message_port = CreateMsgPort(); if (timer_message_port == NULL) return 0; timer_io = (struct timerequest*)CreateExtIO(timer_message_port, sizeof(struct timerequest)); if (timer_io == NULL) return 0; if (OpenDevice( TIMERNAME, UNIT_VBLANK, (struct IORequest *)timer_io, 0L )) return 0; timer_sign = (1L << timer_message_port->mp_SigBit ); // Before bothering with anything else, open the library if ( (CxBase = OpenLibrary((CONST_STRPTR)"commodities.library", 37L)) ) { // Commodities talks to a Commodities application through // an Exec Message port, which the application provides if ( (cx_broker_message_port = CreateMsgPort()) ) { cx_newbroker.nb_Port = cx_broker_message_port; // The commodities.library function CxBroker() adds a // broker to the master list. It takes two arguments, // a pointer to a NewBroker structure and a pointer to // a LONG. The NewBroker structure contains information // to set up the broker. If the second argument is not // NULL, CxBroker will fill it in with an error code. if ( (cx_broker = CxBroker(&cx_newbroker, NULL)) ) { // After it's set up correctly, the broker // has to be activated ActivateCxObj(cx_broker, 1L); cx_sign = (1L << cx_broker_message_port->mp_SigBit); timer_io->tr_node.io_Command = TR_ADDREQUEST; timer_io->tr_time.tv_secs = 0; timer_io->tr_time.tv_micro = 10; SendIO((struct IORequest *)timer_io); BYTE is_running = 1; // the main processing loop BYTE loop = 1; while(loop) { CxMsg *cx_message; ULONG cx_message_id; ULONG cx_message_type; ULONG sign = Wait( timer_sign | cx_sign); if ( (sign & timer_sign) && is_running) { while( GetMsg(timer_message_port)) { numer++; printf("YES: %d ", numer); timer_io->tr_node.io_Command = TR_ADDREQUEST; timer_io->tr_time.tv_secs = 2; timer_io->tr_time.tv_micro = 0; SendIO((struct IORequest *)timer_io); } } if (sign & cx_sign) while(cx_message = (CxMsg*) GetMsg(cx_broker_message_port)) { // Extract necessary information from the CxMessage and return it cx_message_id = CxMsgID(cx_message); cx_message_type = CxMsgType(cx_message); ReplyMsg((struct Message *)cx_message); switch(cx_message_type) { // Commodities has sent a command case CXM_COMMAND: switch(cx_message_id) { case CXCMD_DISABLE: printf("msInternetStatus has been DISABLED.\n"); ActivateCxObj(cx_broker, 0L); is_running = 0; break; case CXCMD_ENABLE: printf("msInternetStatus has been ENABLED.\n"); ActivateCxObj(cx_broker, 1L); is_running = 1; break; case CXCMD_KILL: printf( "msInternetStatus has been REMOVED from the system.\n"); loop = 0L; is_running = 0; break; } break; } } } DeleteCxObj(cx_broker); } DeletePort(cx_broker_message_port); } CloseLibrary(CxBase); } //CloseDevice( (struct IORequest*) timer_io); //DeleteExtIO( (struct IORequest*) timer_io); //DeleteMsgPort(timer_message_port); return 0; } |
![]() |
![]() |
#2 |
Returning fan!
Join Date: Jan 2011
Location: Montréal, QC, Canada
Posts: 1,456
|
Hi Mateusz_s!
Thanks for sharing your code! ![]() It looks good to me, but what happens if sign & timer_sign is not 0 and is_running is 0? Actually, what's the use of is_running in comparison to loop? Cheers! |
![]() |
![]() |
#3 | |
Registered User
Join Date: Jan 2020
Location: Poland
Posts: 182
|
Quote:
A bit corrected and cleaned up version, I think this should be ok. CPU idle is OK. I can enable/disable the activity. Probably the timer cleanup is bugged right now because I got crash after exiting. Code:
#include <proto/exec.h> #include <proto/dos.h> #include <clib/alib_protos.h> #include <clib/commodities_protos.h> #include <libraries/commodities.h> #include <sys/socket.h> #include <proto/socket.h> #include <netinet/in.h> #include <netinet/tcp.h> #include <sys/ioctl.h> #include <stdio.h> // Application name and version. #define APP_NAME "msIntenetStatus" #define APP_VERSION "v0.1" #define APP_DESCRIPTION "Checks Internet connetion in regular time intervals." // Libs struct Library* CxBase = NULL; struct Library* SocketBase = NULL; // Commodity globals struct NewBroker cx_newbroker = { NB_VERSION, // nb_Version - Version of the NewBroker structure (STRPTR)APP_NAME, // nb_Name - Name CX uses to identify this commodity (STRPTR)APP_NAME" "APP_VERSION, // nb_Title - Title of commodity that appears in CXExchange (STRPTR)APP_DESCRIPTION, // nb_Descr - Description of the commodity NBU_UNIQUE, // nb_Unique - Tells CX not to launch new commodity with same name 0, // nb_Flags - Tells CX if this commodity has a window 0, // nb_Pri - This commodity's priority 0, // nb_Port - MsgPort CX talks to 0 // nb_ReservedChannel - reserved for later use }; struct MsgPort *cx_broker_message_port; CxObj *cx_broker; ULONG cx_signal = 0; // --- timer globals --- struct MsgPort* timer_message_port; struct timerequest* timer_io; struct Message *timer_message; struct IOExtSer *reply; ULONG timer_signal = 0; int numer = 0; // Timer functions definitions. int timer_init() { timer_message_port = CreateMsgPort(); if (timer_message_port == NULL) return 0; timer_io = (struct timerequest*)CreateExtIO(timer_message_port, sizeof(struct timerequest)); if (timer_io == NULL) return 0; if (OpenDevice(TIMERNAME, UNIT_VBLANK, (struct IORequest *)timer_io, 0L)) return 0; timer_signal = (1L << timer_message_port->mp_SigBit ); return 1; } void timer_send(ULONG _sec, ULONG _micro) { timer_io->tr_node.io_Command = TR_ADDREQUEST; timer_io->tr_time.tv_micro = _micro; timer_io->tr_time.tv_sec = 0; timer_io->tr_time.tv_secs = _sec; timer_io->tr_time.tv_usec = 0; SendIO((struct IORequest *)timer_io); } void timer_cleanup() { CloseDevice( (struct IORequest*) timer_io); DeleteExtIO( (struct IORequest*) timer_io); DeleteMsgPort(timer_message_port); } int main(void) { // Lets init the timer first. if (!timer_init()) { printf("%s: Error! Couldn't create the timer...", APP_NAME); timer_cleanup(); return 1; } // Before bothering with anything else, open the library. if ( (CxBase = OpenLibrary((CONST_STRPTR)"commodities.library", 37L)) ) { // Commodities talks to a Commodities application through // an Exec Message port, which the application provides if ( (cx_broker_message_port = CreateMsgPort()) ) { cx_newbroker.nb_Port = cx_broker_message_port; // The commodities.library function CxBroker() adds a // broker to the master list. It takes two arguments, // a pointer to a NewBroker structure and a pointer to // a LONG. The NewBroker structure contains information // to set up the broker. If the second argument is not // NULL, CxBroker will fill it in with an error code. if ( (cx_broker = CxBroker(&cx_newbroker, NULL)) ) { // After it's set up correctly, the broker has to be activated. ActivateCxObj(cx_broker, 1L); cx_signal = (1L << cx_broker_message_port->mp_SigBit); // Send first - short interval to timer. timer_send(0, 1); // Enabled/Disabled status. BYTE cx_enabled = 1; // The main processing loop. BYTE cx_loop = 1; while(cx_loop) { CxMsg *cx_message; ULONG cx_message_id; ULONG cx_message_type; // Wait until timer or cx signal appear. Wait( timer_signal | cx_signal); // -------------------------------------------------------------------- // --- If we get the signal from the timer and commodity is enabled, // --- execute our checking routine. // -------------------------------------------------------------------- while(GetMsg(timer_message_port) && cx_enabled) { numer++; printf("YES: %d ", numer); timer_send(2, 0); } // ------------------------------------------------------------- // Commodity msg processing while(cx_message = (CxMsg*)GetMsg(cx_broker_message_port)) { // Extract necessary information from the CxMessage and return it cx_message_id = CxMsgID(cx_message); cx_message_type = CxMsgType(cx_message); ReplyMsg((struct Message *)cx_message); switch(cx_message_type) { // Commodities has sent a command case CXM_COMMAND: switch(cx_message_id) { case CXCMD_DISABLE: ActivateCxObj(cx_broker, 0L); cx_enabled = 0; break; case CXCMD_ENABLE: timer_send(2, 0); ActivateCxObj(cx_broker, 1L); cx_enabled = 1; break; case CXCMD_KILL: cx_loop = 0; cx_enabled = 0; break; } break; } } } DeleteCxObj(cx_broker); } DeletePort(cx_broker_message_port); } CloseLibrary(CxBase); } return 0; } |
|
![]() |
![]() |
#4 |
Registered User
Join Date: Jun 2016
Location: europe
Posts: 1,097
|
You're not calling timer_cleanup() before you exit normally (cx kill msg received), plus you should AbortIO() if it's in progress. Also missing a ReplyMsg() for timer msgs.
|
![]() |
![]() |
#5 | |
Registered User
Join Date: Jan 2020
Location: Poland
Posts: 182
|
Quote:
I also saw this thread: https://eab.abime.net/showthread.php?p=1458225 and read info from @Thomas Richter and @thomas - and I changed GetMsg() into WaitIO() as for ReplyMsg() I search thru couple of exmapples and no one used it.. ![]() Code:
#include <proto/exec.h> #include <proto/dos.h> #include <clib/alib_protos.h> #include <clib/commodities_protos.h> #include <libraries/commodities.h> #include <sys/socket.h> #include <proto/socket.h> #include <netinet/in.h> #include <netinet/tcp.h> #include <sys/ioctl.h> #include <stdio.h> // Application name and version. #define APP_NAME "msIntenetStatus" #define APP_VERSION "v0.1" #define APP_DESCRIPTION "Checks Internet connection status." // Version that appears in Icon->Information. static const char *APP_info_version = "$VER: Version "APP_VERSION; // Libs. struct Library* CxBase = NULL; struct Library* SocketBase = NULL; // Commodity globals. struct NewBroker cx_newbroker = { NB_VERSION, // nb_Version - Version of the NewBroker structure (STRPTR)APP_NAME, // nb_Name - Name CX uses to identify this commodity (STRPTR)APP_NAME" "APP_VERSION, // nb_Title - Title of commodity that appears in CXExchange (STRPTR)APP_DESCRIPTION, // nb_Descr - Description of the commodity NBU_UNIQUE, // nb_Unique - Tells CX not to launch new commodity with same name 0, // nb_Flags - Tells CX if this commodity has a window 0, // nb_Pri - This commodity's priority 0, // nb_Port - MsgPort CX talks to 0 // nb_ReservedChannel - reserved for later use }; struct MsgPort* cx_broker_message_port; CxObj* cx_broker; ULONG cx_signal; // Timer globals. struct MsgPort* timer_message_port; struct timerequest* timer_io; ULONG timer_signal; int numer = 0; // Timer functions definitions. int timer_init() { timer_message_port = CreateMsgPort(); if (timer_message_port == NULL) return 0; timer_io = (struct timerequest*)CreateExtIO(timer_message_port, sizeof(struct timerequest)); if (timer_io == NULL) return 0; if (OpenDevice(TIMERNAME, UNIT_VBLANK, (struct IORequest *)timer_io, 0L)) return 0; timer_signal = (1L << timer_message_port->mp_SigBit ); return 1; } void timer_send(ULONG _sec, ULONG _micro) { timer_io->tr_node.io_Command = TR_ADDREQUEST; timer_io->tr_time.tv_micro = _micro; timer_io->tr_time.tv_sec = 0; timer_io->tr_time.tv_secs = _sec; timer_io->tr_time.tv_usec = 0; SendIO((struct IORequest *)timer_io); } void timer_cleanup() { // All I/O requests must be complete before CloseDevice(). AbortIO(timer_io); // Clean up. WaitIO(timer_io); CloseDevice( (struct IORequest*) timer_io); DeleteExtIO( (struct IORequest*) timer_io); DeleteMsgPort(timer_message_port); } // Entry point. int main(void) { // Lets init the timer first. if (!timer_init()) { printf("%s: Error! Couldn't create the timer...", APP_NAME); timer_cleanup(); return 1; } // Before bothering with anything else, open the library. if ( (CxBase = OpenLibrary((CONST_STRPTR)"commodities.library", 37L)) ) { // Commodities talks to a Commodities application through // an Exec Message port, which the application provides if ( (cx_broker_message_port = CreateMsgPort()) ) { cx_newbroker.nb_Port = cx_broker_message_port; // The commodities.library function CxBroker() adds a // broker to the master list. It takes two arguments, // a pointer to a NewBroker structure and a pointer to // a LONG. The NewBroker structure contains information // to set up the broker. If the second argument is not // NULL, CxBroker will fill it in with an error code. if ( (cx_broker = CxBroker(&cx_newbroker, NULL)) ) { // After it's set up correctly, the broker has to be activated. ActivateCxObj(cx_broker, 1L); cx_signal = (1L << cx_broker_message_port->mp_SigBit); // Send first - short interval to timer. timer_send(0, 1); // Enabled/Disabled status. BYTE cx_enabled = 1; // The main processing loop. BYTE cx_loop = 1; while(cx_loop) { CxMsg *cx_message; ULONG cx_message_id; ULONG cx_message_type; // Wait until timer or cx signal appear. ULONG signal = Wait(cx_signal | timer_signal); // -------------------------------------------------------------------- // --- If we get the signal from the timer and commodity is enabled, // --- execute our checking routine. // -------------------------------------------------------------------- if ( (signal & timer_signal) && cx_enabled) { // Using WaitIO() to handle request instead of GetMsg(). WaitIO(timer_io); numer++; printf("YES: %d ", numer); timer_send(2, 0); } // ------------------------------------------------------------- // Commodity msg processing if ( signal & cx_signal ) { while(cx_message = (CxMsg*)GetMsg(cx_broker_message_port)) { // Extract necessary information from the CxMessage and return it cx_message_id = CxMsgID(cx_message); cx_message_type = CxMsgType(cx_message); ReplyMsg((struct Message*)cx_message); switch(cx_message_type) { // Commodities has sent a command case CXM_COMMAND: switch(cx_message_id) { case CXCMD_DISABLE: AbortIO(timer_io); WaitIO(timer_io); ActivateCxObj(cx_broker, 0L); cx_enabled = 0; break; case CXCMD_ENABLE: timer_send(2, 0); ActivateCxObj(cx_broker, 1L); cx_enabled = 1; break; case CXCMD_KILL: cx_loop = 0; cx_enabled = 0; break; } break; } } } } DeleteCxObj(cx_broker); } DeletePort(cx_broker_message_port); } CloseLibrary(CxBase); } // Clean up timer. timer_cleanup(); return 0; } |
|
![]() |
![]() |
#6 |
Registered User
Join Date: Jun 2016
Location: europe
Posts: 1,097
|
|
![]() |
![]() |
#7 |
Returning fan!
Join Date: Jan 2011
Location: Montréal, QC, Canada
Posts: 1,456
|
Hi Mateusz_s!
Looks good to me! For what it's worth, this is my timer code for AmigaMapPing, which looks very much like your code ![]() Thanks for sharing your code! |
![]() |
Currently Active Users Viewing This Thread: 1 (0 members and 1 guests) | |
Thread Tools | |
![]() |
||||
Thread | Thread Starter | Forum | Replies | Last Post |
Can this application be a Commodity? | mateusz_s | Coders. System | 2 | 19 December 2021 01:54 |
Helpful commodity | idrougge | Coders. Blitz Basic | 0 | 14 December 2015 19:51 |
Key logger Commodity with Source Code? | Sim085 | support.Other | 10 | 06 July 2015 01:09 |
Translators for a little commodity wanted | bubbob42 | support.Apps | 12 | 29 May 2014 16:40 |
Looking for Caps2Ctrl commodity | kolla | support.Apps | 7 | 24 October 2009 17:14 |
|
|