You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/02/18 12:52:56 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #297:
Simplify syslog
xiaoxiang781216 opened a new pull request #297: Simplify syslog
URL: https://github.com/apache/incubator-nuttx/pull/297
This patch set simplify syslog/serial subsytem by:
1.Remove lowconsole and ramconsole since syslog console can do the same thing
2.Remove g_ramlog_syslog_channel since g_default_channel can do the same thing
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #297: Simplify
syslog
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #297: Simplify syslog
URL: https://github.com/apache/incubator-nuttx/pull/297#issuecomment-587482647
> This seems to depend on the serial device being the console and upon the syslog device being the serial console. That is not always the case. I don't understand the change fully. What I see does not appear to be correct.
>
> Most of the commits look good, basically removing RAM and SYSLOG consoles. and serial lowconsole. But I don't understand why the serial initializatin logic was removed. That does not make sense to me.
>
@patacongo do you mean this patch?
https://github.com/apache/incubator-nuttx/pull/297/commits/9dd2bf7cd3bad9337a989ccae7b5b39a71e30ec1:
arch/z80: Remove z80_lowserial_initialize related stuff
since nobody call this function at all
> It would be nice if we could divide this up into separate changes and not hold up the good changes based on some that seem not so good (but perhaps I don't understand them).
Could you point out which patch isn't good than I can remove it from patchset or make more explanation?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #297: Simplify syslog
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #297: Simplify syslog
URL: https://github.com/apache/incubator-nuttx/pull/297#issuecomment-587654263
A note about "drivers/serial: Remove the lowconsole driver": This is a remnant of some old minimum footprint logic. You are right, it is not of value in the modern NuttX architecture.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #297: Simplify
syslog
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #297: Simplify syslog
URL: https://github.com/apache/incubator-nuttx/pull/297#issuecomment-587696571
> A note about "ramlog: Remove all ramlog_consoleinit related code": The purpose of this was to run "headless", then connect later say via telnetd. Then you could view all cosonsole output in the ramlog. However, I am not aware that anyone has ever used that feature.
This case still support: user can enable both CONFIG_RAMLOG_SYSLOG and CONFIG_CONSOLE_SYSLOG:
1.syslog send to ramlog with CONFIG_RAMLOG_SYSLOG
2.printf send to ramlog with CONFIG_CONSOLE_SYSLOG
3.dmesg in telnet receive log from /dev/ramlog(registered due to CONFIG_RAMLOG_SYSLOG)
And this patch set also fix one issue in this case:
ramlog_consoleinit register /dev/console with g_ramlogfops and ramlog_read return the ram buffer content which actually come from syslog/printf, then most likely will trigger nsh run the wrong and random command.
With this patch, /dev/console back up with g_consoleops and syslog_console_read is an empty funtion, then stop the loopback path totally.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #297: Simplify syslog
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #297: Simplify syslog
URL: https://github.com/apache/incubator-nuttx/pull/297#issuecomment-587658213
A note about "ramlog: Remove all ramlog_consoleinit related code": The purpose of this was to run "headless", then connect later say via telnetd. Then you could view all cosonsole output in the ramlog. However, I am not aware that anyone has ever used that feature.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo edited a comment on issue #297:
Simplify syslog
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #297: Simplify syslog
URL: https://github.com/apache/incubator-nuttx/pull/297#issuecomment-587459155
This seems to depend on the serial device being the console and upon the syslog device being the serial console. That is not always the case. I don't understand the change fully. What I see does not appear to be correct.
Most of the commits look good, basically removing RAM and SYSLOG consoles. and serial lowconsole. But I don't understand why the serial initializatin logic was removed. That does not make sense to me.
It would be nice if we could divide this up into separate changes and not hold up the good changes based on some that seem not so good (but perhaps I don't understand them).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo merged pull request #297: Simplify
syslog
Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #297: Simplify syslog
URL: https://github.com/apache/incubator-nuttx/pull/297
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #297: Simplify syslog
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #297: Simplify syslog
URL: https://github.com/apache/incubator-nuttx/pull/297#issuecomment-587459155
This seems to depend on the serial device being the console and upon the syslog device being the serial console. That is not always the case. I don't understand the change fully. What I see does not appear to be correct.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] patacongo commented on issue #297: Simplify syslog
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #297: Simplify syslog
URL: https://github.com/apache/incubator-nuttx/pull/297#issuecomment-587628413
This is too much to review all at once on line. I am going to bring the commits onto a branch where I can properly review them. I will merge them one-at-a-time when I complete the review. In doing this, there is a risk because it will not be possible to update the PR if I find any issues.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services