| Message ID | 20210715180450.21441-1-JPEWhacker@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | mongoose: Ignore PROGRESS messages | expand |
On 15.07.21 20:04, Joshua Watt wrote: > PROGRESS messages should not be passed to websocket clients since they > show up as "UNKNOWN". If a client want progress messages, a dedicated > JSON message should be used to transmit the information. > Correct ! > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> > --- > mongoose/mongoose_interface.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c > index b6da93d..290f33a 100644 > --- a/mongoose/mongoose_interface.c > +++ b/mongoose/mongoose_interface.c > @@ -200,6 +200,9 @@ static void *broadcast_progress_thread(void *data) > if (ret != sizeof(msg)) > return NULL; > > + if (msg.status == PROGRESS) > + continue; > + > if (msg.status != status || msg.status == FAILURE) { > status = msg.status; > > Acked-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
Hi Joshua, On 15.07.21 20:04, Joshua Watt wrote: > PROGRESS messages should not be passed to websocket clients since they > show up as "UNKNOWN". If a client want progress messages, a dedicated > JSON message should be used to transmit the information. > > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> > --- > mongoose/mongoose_interface.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c > index b6da93d..290f33a 100644 > --- a/mongoose/mongoose_interface.c > +++ b/mongoose/mongoose_interface.c > @@ -200,6 +200,9 @@ static void *broadcast_progress_thread(void *data) > if (ret != sizeof(msg)) > return NULL; > > + if (msg.status == PROGRESS) > + continue; > + > if (msg.status != status || msg.status == FAILURE) { > status = msg.status; > > Reactivating an old thread due to a regression issue. I was debugging a weird effece: when I update from Mongoose, I do not see the progress bar moving. Progress messages are sent internally, because swupdate-progress works as expected. I started bisect from a working version, and at the end this patch is responsible for the behavior. I can state that it is really responsible for the issue (well, in between there is an update for the mongoose server, so there are several possible causes), because reverting on current TOT, issue disappears. At the moment of the review, patch looked me ok, but it is clear I had not tested enough and I have not seen the issue. Anyway, without PROGRESS messsages, the bar oi the browser does not move (web-app is looking for thsoe messages). It is now not clear to me where the UNKNOWN is seen, as the web-app expects and parses these messages. II am in doubt if this patch should be reworked (then I ask you) due to the unknown message, or simply reverted (this is my first thought). What do you think ? Best regards, Stefano
On Fri, Nov 26, 2021, 4:11 AM Stefano Babic <sbabic@denx.de> wrote: > Hi Joshua, > > On 15.07.21 20:04, Joshua Watt wrote: > > PROGRESS messages should not be passed to websocket clients since they > > show up as "UNKNOWN". If a client want progress messages, a dedicated > > JSON message should be used to transmit the information. > > > > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> > > --- > > mongoose/mongoose_interface.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/mongoose/mongoose_interface.c > b/mongoose/mongoose_interface.c > > index b6da93d..290f33a 100644 > > --- a/mongoose/mongoose_interface.c > > +++ b/mongoose/mongoose_interface.c > > @@ -200,6 +200,9 @@ static void *broadcast_progress_thread(void *data) > > if (ret != sizeof(msg)) > > return NULL; > > > > + if (msg.status == PROGRESS) > > + continue; > > + > > if (msg.status != status || msg.status == FAILURE) { > > status = msg.status; > > > > > > Reactivating an old thread due to a regression issue. I was debugging a > weird effece: when I update from Mongoose, I do not see the progress bar > moving. Progress messages are sent internally, because swupdate-progress > works as expected. I started bisect from a working version, and at the > end this patch is responsible for the behavior. I can state that it is > really responsible for the issue (well, in between there is an update > for the mongoose server, so there are several possible causes), because > reverting on current TOT, issue disappears. > > At the moment of the review, patch looked me ok, but it is clear I had > not tested enough and I have not seen the issue. Anyway, without > PROGRESS messsages, the bar oi the browser does not move (web-app is > looking for thsoe messages). It is now not clear to me where the UNKNOWN > is seen, as the web-app expects and parses these messages. > > II am in doubt if this patch should be reworked (then I ask you) due to > the unknown message, or simply reverted (this is my first thought). What > do you think ? > I suspect reworking would be best to make the progress messages their own explicit message type, instead of assuming UNKNOWN is a progress message. I can look at that next week if that is ok? > Best regards, > Stefano > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > ===================================================================== >
Hi Joshua, On 26.11.21 17:32, Joshua Watt wrote: > > > On Fri, Nov 26, 2021, 4:11 AM Stefano Babic <sbabic@denx.de > <mailto:sbabic@denx.de>> wrote: > > Hi Joshua, > > On 15.07.21 20:04, Joshua Watt wrote: > > PROGRESS messages should not be passed to websocket clients since > they > > show up as "UNKNOWN". If a client want progress messages, a dedicated > > JSON message should be used to transmit the information. > > > > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com > <mailto:JPEWhacker@gmail.com>> > > --- > > mongoose/mongoose_interface.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/mongoose/mongoose_interface.c > b/mongoose/mongoose_interface.c > > index b6da93d..290f33a 100644 > > --- a/mongoose/mongoose_interface.c > > +++ b/mongoose/mongoose_interface.c > > @@ -200,6 +200,9 @@ static void *broadcast_progress_thread(void > *data) > > if (ret != sizeof(msg)) > > return NULL; > > > > + if (msg.status == PROGRESS) > > + continue; > > + > > if (msg.status != status || msg.status == FAILURE) { > > status = msg.status; > > > > > > Reactivating an old thread due to a regression issue. I was debugging a > weird effece: when I update from Mongoose, I do not see the progress > bar > moving. Progress messages are sent internally, because > swupdate-progress > works as expected. I started bisect from a working version, and at the > end this patch is responsible for the behavior. I can state that it is > really responsible for the issue (well, in between there is an update > for the mongoose server, so there are several possible causes), because > reverting on current TOT, issue disappears. > > At the moment of the review, patch looked me ok, but it is clear I had > not tested enough and I have not seen the issue. Anyway, without > PROGRESS messsages, the bar oi the browser does not move (web-app is > looking for thsoe messages). It is now not clear to me where the > UNKNOWN > is seen, as the web-app expects and parses these messages. > > II am in doubt if this patch should be reworked (then I ask you) due to > the unknown message, or simply reverted (this is my first thought). > What > do you think ? > > > I suspect reworking would be best to make the progress messages their > own explicit message type, instead of assuming UNKNOWN is a progress > message. I can look at that next week if that is ok? Fine with me, thanks ! Best regards, Stefano > > > > Best regards, > Stefano > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > sbabic@denx.de <mailto:sbabic@denx.de> > ===================================================================== >
On Fri, Nov 26, 2021 at 11:40 AM Stefano Babic <sbabic@denx.de> wrote: > > Hi Joshua, > > On 26.11.21 17:32, Joshua Watt wrote: > > > > > > On Fri, Nov 26, 2021, 4:11 AM Stefano Babic <sbabic@denx.de > > <mailto:sbabic@denx.de>> wrote: > > > > Hi Joshua, > > > > On 15.07.21 20:04, Joshua Watt wrote: > > > PROGRESS messages should not be passed to websocket clients since > > they > > > show up as "UNKNOWN". If a client want progress messages, a dedicated > > > JSON message should be used to transmit the information. > > > > > > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com > > <mailto:JPEWhacker@gmail.com>> > > > --- > > > mongoose/mongoose_interface.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/mongoose/mongoose_interface.c > > b/mongoose/mongoose_interface.c > > > index b6da93d..290f33a 100644 > > > --- a/mongoose/mongoose_interface.c > > > +++ b/mongoose/mongoose_interface.c > > > @@ -200,6 +200,9 @@ static void *broadcast_progress_thread(void > > *data) > > > if (ret != sizeof(msg)) > > > return NULL; > > > > > > + if (msg.status == PROGRESS) > > > + continue; > > > + > > > if (msg.status != status || msg.status == FAILURE) { > > > status = msg.status; > > > > > > > > > > Reactivating an old thread due to a regression issue. I was debugging a > > weird effece: when I update from Mongoose, I do not see the progress > > bar > > moving. Progress messages are sent internally, because > > swupdate-progress > > works as expected. I started bisect from a working version, and at the > > end this patch is responsible for the behavior. I can state that it is > > really responsible for the issue (well, in between there is an update > > for the mongoose server, so there are several possible causes), because > > reverting on current TOT, issue disappears. > > > > At the moment of the review, patch looked me ok, but it is clear I had > > not tested enough and I have not seen the issue. Anyway, without > > PROGRESS messsages, the bar oi the browser does not move (web-app is > > looking for thsoe messages). It is now not clear to me where the > > UNKNOWN > > is seen, as the web-app expects and parses these messages. > > > > II am in doubt if this patch should be reworked (then I ask you) due to > > the unknown message, or simply reverted (this is my first thought). > > What > > do you think ? > > > > > > I suspect reworking would be best to make the progress messages their > > own explicit message type, instead of assuming UNKNOWN is a progress > > message. I can look at that next week if that is ok? > > Fine with me, thanks ! I just sent a patch to the list that I believe will fix this problem; please give it a try to make sure it's behaving correctly under the conditions you were seeing. Thanks > > Best regards, > Stefano > > > > > > > > > Best regards, > > Stefano > > > > -- > > ===================================================================== > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > > sbabic@denx.de <mailto:sbabic@denx.de> > > ===================================================================== > > > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > =====================================================================
On 30.11.21 13:22, Joshua Watt wrote: > On Fri, Nov 26, 2021 at 11:40 AM Stefano Babic <sbabic@denx.de> wrote: >> >> Hi Joshua, >> >> On 26.11.21 17:32, Joshua Watt wrote: >>> >>> >>> On Fri, Nov 26, 2021, 4:11 AM Stefano Babic <sbabic@denx.de >>> <mailto:sbabic@denx.de>> wrote: >>> >>> Hi Joshua, >>> >>> On 15.07.21 20:04, Joshua Watt wrote: >>> > PROGRESS messages should not be passed to websocket clients since >>> they >>> > show up as "UNKNOWN". If a client want progress messages, a dedicated >>> > JSON message should be used to transmit the information. >>> > >>> > Signed-off-by: Joshua Watt <JPEWhacker@gmail.com >>> <mailto:JPEWhacker@gmail.com>> >>> > --- >>> > mongoose/mongoose_interface.c | 3 +++ >>> > 1 file changed, 3 insertions(+) >>> > >>> > diff --git a/mongoose/mongoose_interface.c >>> b/mongoose/mongoose_interface.c >>> > index b6da93d..290f33a 100644 >>> > --- a/mongoose/mongoose_interface.c >>> > +++ b/mongoose/mongoose_interface.c >>> > @@ -200,6 +200,9 @@ static void *broadcast_progress_thread(void >>> *data) >>> > if (ret != sizeof(msg)) >>> > return NULL; >>> > >>> > + if (msg.status == PROGRESS) >>> > + continue; >>> > + >>> > if (msg.status != status || msg.status == FAILURE) { >>> > status = msg.status; >>> > >>> > >>> >>> Reactivating an old thread due to a regression issue. I was debugging a >>> weird effece: when I update from Mongoose, I do not see the progress >>> bar >>> moving. Progress messages are sent internally, because >>> swupdate-progress >>> works as expected. I started bisect from a working version, and at the >>> end this patch is responsible for the behavior. I can state that it is >>> really responsible for the issue (well, in between there is an update >>> for the mongoose server, so there are several possible causes), because >>> reverting on current TOT, issue disappears. >>> >>> At the moment of the review, patch looked me ok, but it is clear I had >>> not tested enough and I have not seen the issue. Anyway, without >>> PROGRESS messsages, the bar oi the browser does not move (web-app is >>> looking for thsoe messages). It is now not clear to me where the >>> UNKNOWN >>> is seen, as the web-app expects and parses these messages. >>> >>> II am in doubt if this patch should be reworked (then I ask you) due to >>> the unknown message, or simply reverted (this is my first thought). >>> What >>> do you think ? >>> >>> >>> I suspect reworking would be best to make the progress messages their >>> own explicit message type, instead of assuming UNKNOWN is a progress >>> message. I can look at that next week if that is ok? >> >> Fine with me, thanks ! > > I just sent a patch to the list that I believe will fix this problem; > please give it a try to make sure it's behaving correctly under the > conditions you were seeing. Thanks, I will test it and I will be back to you ! Regards, Stefano > > Thanks > >> >> Best regards, >> Stefano >> >>> >>> >>> >>> Best regards, >>> Stefano >>> >>> -- >>> ===================================================================== >>> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >>> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: >>> sbabic@denx.de <mailto:sbabic@denx.de> >>> ===================================================================== >>> >> >> -- >> ===================================================================== >> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de >> ===================================================================== >
diff --git a/mongoose/mongoose_interface.c b/mongoose/mongoose_interface.c index b6da93d..290f33a 100644 --- a/mongoose/mongoose_interface.c +++ b/mongoose/mongoose_interface.c @@ -200,6 +200,9 @@ static void *broadcast_progress_thread(void *data) if (ret != sizeof(msg)) return NULL; + if (msg.status == PROGRESS) + continue; + if (msg.status != status || msg.status == FAILURE) { status = msg.status;
PROGRESS messages should not be passed to websocket clients since they show up as "UNKNOWN". If a client want progress messages, a dedicated JSON message should be used to transmit the information. Signed-off-by: Joshua Watt <JPEWhacker@gmail.com> --- mongoose/mongoose_interface.c | 3 +++ 1 file changed, 3 insertions(+)