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

Yardforce 500b #12

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Conversation

jeremysalwen
Copy link

(Continuation of Janrupf's pull request)

Additionally fix the potential bug and minor cleanups.

@jeremysalwen jeremysalwen mentioned this pull request May 11, 2024
@Janrupf
Copy link

Janrupf commented May 24, 2024

@cedbossneo Have you gotten around to testing this version of the 500b support?

@wjcloudy
Copy link

wjcloudy commented Jun 6, 2024

@jeremysalwen @Janrupf It's been a while but there were some additional fixes from @benjaminlundgreen in the adc_dma branch that I'm not sure if have been picked up (or resolved by other means)
https://github.com/benjaminlundgreen/Mowgli/commits/500b_adc_dma/

I can't remember the reason entirely - was around the clock speed, DMA and I think was fixing some ADC reads... (I'm using that version with everything working)

@benjaminlundgreen
Copy link

I think the DMA functionality can be skipped, that was a wild goose chase. There are however some corrections to get the mow motor NTC working, and also add battery ntcs. benjaminlundgreen@d9cc1b2
Personally I'm not a fan of having branches for variants, so I'd prefer to merge 500b into main repo in a nice way, but that requires putting in some time which i do not have at the moment.

@brupje
Copy link

brupje commented Jun 10, 2024

I had to add platformio/framework-stm32cubef4@^1.26.2 to platformio.ini and comment out #include "proxy_inc/stm32f4/startup_stm32f4xx.s" in startup_stm32f.s to get the repository of @jeremysalwen to compile. If I didn't do the first I get an undefined reference to `HAL_UARTEx_ReceiveToIdle_DMA' , which https://community.st.com/t5/stm32cubeide-mcus/why-is-there-no-quot-stm32f4xx-hal-uart-ex-c-quot-hal-library-in/td-p/220258 refers to version1.26.2 as the fix.

@jeremysalwen
Copy link
Author

@brupje Coud you explain a bit more? Can you compile @Janrupf's https://github.com/Janrupf/Mowgli/tree/yardforce-500b branch without modification?

@brupje
Copy link

brupje commented Jun 11, 2024

@jeremysalwen : thanks for your reply. No, I get the same error with @Janrupf 's repository:

.platformio/packages/[email protected]/bin/../lib/gcc/arm-none-eabi/9.3.1/../../../../arm-none-eabi/bin/ld: .pio/build/Yardforce500B/src/panel.o: in function PANEL_Init': panel.c:(.text.PANEL_Init+0x208): undefined reference to HAL_UARTEx_ReceiveToIdle_DMA'
collect2: error: ld returned 1 exit status
*** [.pio/build/Yardforce500B/firmware.elf] Error 1

adding the line platformio/framework-stm32cubef4@^1.26.2 to platformio.ini results me in getting:

.pio/build/Yardforce500B/src/startup_stm32f.o: in function Default_Handler': (.text.Default_Handler+0x0): multiple definition of Default_Handler'; .pio/build/Yardforce500B/src/proxy_inc/stm32f4/startup_stm32f4xx.o:(.text.Default_Handler+0x0): first defined here

Commenting
//#include "proxy_inc/stm32f4/startup_stm32f4xx.s"
allows me to compile and upload.

@Janrupf
Copy link

Janrupf commented Jun 11, 2024

Oh yeah, not having #include "proxy_inc/stm32f4/startup_stm32f4xx.s" is actually the way its intended to be. The entire proxy_inc thing is kind of a hack (though platformio is a bit too inflexible to completely get rid of it without changing the folder structure). So it now erroring is kind of actually meaning that it was fixed.

So yeah, adding the line to the platformio.ini and removing the include is indeed the correct way

@jeremysalwen
Copy link
Author

Okay, thanks @Janrupf and @brupje I don't have access to my PC to update the branch right now but I will do it next week.

@jeremysalwen
Copy link
Author

Hmm, I can't reproduce the failing build with the latest platformIO. It seems to compile fine, with or without the include and with or without the platform.ini change. Do you have an idea what might be different on your system?

Looking at the code, I'm also not clear about what the suggested fix is. Are we just completely removing the proxy_inc/stm32f4/startup_stm32f4xx.s, proxy_inc/stm32f4/startup_stm32f1xx.s, and startup_stm32f.s files?

@Janrupf
Copy link

Janrupf commented Jun 20, 2024

startup_stm32f1xx.s is empty anyway, only there for consistency. The thing is, the code which is in the startup_stm32f4xx.s file is copied from the STM32 SDK. PlatformIO was supposed to auto generate/include that file, but it didn't, so I added it manually. Somehow that seems to work now - the file was a workaround for that issue. Having both the in-tree file and the one PlatformIO supplies should lead to linkage errors. Anyway, if it compiles without it, the startup* include hack files can be removed

jeremysalwen added a commit to jeremysalwen/Mowgli that referenced this pull request Jun 26, 2024
jeremysalwen added a commit to jeremysalwen/Mowgli that referenced this pull request Jun 26, 2024
@jeremysalwen
Copy link
Author

Okay, I have deleted the extra assembly files @brupje can you check that it works for you?

@brupje
Copy link

brupje commented Jun 26, 2024

It compiles fine for me. Removing the startup files also fixed the requirement for platformio/framework-stm32cubef4@^1.26.2 in platform.ini. It compiles now happily on 1.25.2, so that is great.

Well almost happily:
arm-none-eabi/bin/ld: warning: cannot find entry symbol Reset_Handler; defaulting to 0000000008000000
Does not look great, but not sure if it breaks anything.

Compiling for the yard500 instead of the 500b does not give this warning.

EDIT: this is on top of the compilation log:
Warning! Cannot find the default startup file for stm32f401vct6. Ignore this warning if the startup code is part of your project.

It does look important.

EDIT2: Reverting back does still give me "Warning! Cannot find the default startup file for stm32f401vct6. Ignore this warning if the startup code is part of your project." , but not the "arm-none-eabi/bin/ld: warning: cannot find entry symbol Reset_Handler; defaulting to 0000000008000000"

Final edit: Ok, I think what is happening is that startup_stm32f4xx.s is required, but somehow it is already included without reference from startup_stm32f.s. Hence, commenting out "#include "proxy_inc/stm32f4/startup_stm32f4xx.s" fixed the double inclusion. But removing the entire file startup_stm32f4xx.s from the project is not accepted by the linker. So only removing startup_stm32f4xx.s would have fixed the issue for me I guess. I am really not sure how the file is included from a subdirectory proxy_inc/stm32f4 and not conflicting with stm32f1/startup_stm32f1xx.s. But maybe through stm32f_it.c that includes "#include "proxy_inc/stm32f4/stm32f4xx_it.c""

@jeremysalwen
Copy link
Author

Okay, new theory:

build_src_filter =
	+<*>
	-<.git/>
	-<.svn/>
	-<proxy_inc/**/*.c>

Note how it doesn't exclude the proxy_inc assembly files. So the issue might be the same assembly file being compiled twice? Unfortunately just filtering out the proxy_inc assembly files gives the same linker warning about Reset_Handler...

benjaminlundgreen and others added 7 commits July 29, 2024 00:32
This was possibly the bug that caused the 500b branch not to work on the 500 classic?
Root cause:
1. .s files are not processed by the preprocessor, since they are passed directly to the assembler.  Only .S files are processed by gcc, which uses the preprocessor before sending to the assembler.  Thus the #includes in startup_stdm32f.s had no effect.
2. The proxy_inc/**/*.s files were accidentally *not* filtered from the sources like the .c files in the same directory.  Thus the startup code for both boards was unconditionally included.
3. The two bugs cancelled each other out for the 500b, resulting in the correct startup code being built.  However, it was incorrectly including the 500b's startup code in the non-500b build.
@jeremysalwen
Copy link
Author

Okay, I think I have solved it. Rebased on the latest main. @brupje please test!

Root cause:
1. .s files are not processed by the preprocessor, since they are passed directly to the assembler. Only .S files are processed by gcc, which uses the preprocessor before sending to the assembler. Thus the #includes in startup_stdm32f.s had no effect.
2. The proxy_inc/**/*.s files were accidentally not filtered from the sources like the .c files in the same directory. Thus the startup code for both boards was unconditionally included.
3. The two bugs cancelled each other out for the 500b, resulting in the correct startup code being built. However, it was incorrectly including the 500b's startup code in the non-500b build.

@juditech3D
Copy link

juditech3D commented Aug 2, 2024

Bonjour je teste la dernière mise a jour pour 500b ( firmware et docker mis a jour), la plupart des infos ne remonte plus !
et ne s'affiche plus forcément sur l'app et dashboard.

Capture d'écran 2024-08-02 020222

Hello, I am testing the latest update for 500b (firmware and docker updated), most of the information is no longer available! and is no longer necessarily displayed on the app and dashboard.

@danou07
Copy link

danou07 commented Aug 2, 2024

Bonjour,
J'ai également un problème avec le 500B, je n'ai plus aucune valeur qui remonte dans mowgli sauf le gps et parfois l'imu.


Hi,
I also have a problem with the 500B, I no longer have any values ​​reported in mowgli except the GPS and sometimes the IMU.

Capture d'écran 2024-08-02 131755

@jeremysalwen
Copy link
Author

Hmm, it's working for me. Have you tried restarting the docker containers?

docker compose down
docker compose up -d

I have found that openmower has been flaky lately, and restarting it can fix it.

If you are getting the IMU showing up, that means OpenMower is able to communicate with the mowgli board. You can check that it is properly generating the new status message by running utils/status.sh. If it has the "mow_enabled" field, then you know the new mowgli firmware is working properly, and it's probably a problem with your OpenMower stack or the openmower-gui.

@danou07
Copy link

danou07 commented Aug 3, 2024

I tried the commands:
docker compose down
docker compose up -d
but that didn't change anything.

I also completely reinstalled the OS and OpenMower, the problem remains the same.
if I run the command: utils/status.sh.
I have the message in the screenshot.

Capture d'écran 2024-08-03 142041

@jeremysalwen
Copy link
Author

jeremysalwen commented Aug 3, 2024

Okay so it looks like the status topic is not being populated. Can you check the imu? If it's not showing up, try catting /dev/mowgli and see if anything is showing up.

Edit: Also I should be clear I do not think these problems are caused by this pull request, since the only change is a rebase and minor change to the build file. If you think it was a recent code change you could checkout before the last commit and see if that works for you.

@brupje
Copy link

brupje commented Aug 5, 2024

Okay, I think I have solved it. Rebased on the latest main. @brupje please test!

Root cause: 1. .s files are not processed by the preprocessor, since they are passed directly to the assembler. Only .S files are processed by gcc, which uses the preprocessor before sending to the assembler. Thus the #includes in startup_stdm32f.s had no effect. 2. The proxy_inc/**/*.s files were accidentally not filtered from the sources like the .c files in the same directory. Thus the startup code for both boards was unconditionally included. 3. The two bugs cancelled each other out for the 500b, resulting in the correct startup code being built. However, it was incorrectly including the 500b's startup code in the non-500b build.

Sorry for the delay, currently battling an overheating STM32 chip. For some reason the chip seems to overheat and draw too much power

Still fails to build without the line platformio/framework-stm32cubef4@^1.26.2 in platform_packages. Other than that it looks fine.

@juditech3D
Copy link

Bonjour, j'ai refait mon ssd, mis ajour les dockers et mis a jour le firmware et toujours aucune remonté, et j'ai ça dans les logs :

I've rebuilt my ssd, updated the dockers and updated the firmware and still no response, and I have this in the logs:

mowgli-openmower | [WARN] [1723029858.621332]: Expected Checksum: 0x97E62D
mowgli-openmower | [WARN] [1723029858.624943]: Actual Checksum: 0x6B845A
mowgli-openmower | [WARN] [1723029877.718543]: Found packet, but checksums didn't match
mowgli-openmower | [WARN] [1723029877.725140]: Expected Checksum: 0x2DE106
mowgli-openmower | [WARN] [1723029877.731086]: Actual Checksum: 0xCFBC12
openmower-gui | [2024/08/07 11:25:12] [ERROR] subscriber '/mower/status' got an error: Client [/openmower-gui] wants topic /mower/status to have datatype/md5sum [mower_msgs/Status/1e002de26b9c7d2ad33d3b887ae6ad16], but our version has [mower_msgs/Status/33cd1b298baa54308c3c9343754a34fc]. Dropping connection.
mowgli-openmower | [WARN] [1723029913.517618]: Found packet, but checksums didn't match
mowgli-openmower | [WARN] [1723029913.521987]: Expected Checksum: 0x7B5688
mowgli-openmower | [WARN] [1723029913.526111]: Actual Checksum: 0x806462
openmower-gui | [2024/08/07 11:25:17] [ERROR] subscriber '/mower/status' got an error: Client [/openmower-gui] wants topic /mower/status to have datatype/md5sum [mower_msgs/Status/1e002de26b9c7d2ad33d3b887ae6ad16], but our version has [mower_msgs/Status/33cd1b298baa54308c3c9343754a34fc]. Dropping connection.
openmower-gui | [2024/08/07 11:25:22] [ERROR] subscriber '/mower/status' got an error: Client [/openmower-gui] wants topic /mower/status to have datatype/md5sum [mower_msgs/Status/1e002de26b9c7d2ad33d3b887ae6ad16], but our version has [mower_msgs/Status/33cd1b298baa54308c3c9343754a34fc]. Dropping connection.
openmower-gui | [2024/08/07 11:25:27] [ERROR] subscriber '/mower/status' got an error: Client [/openmower-gui] wants topic /mower/status to have datatype/md5sum [mower_msgs/Status/1e002de26b9c7d2ad33d3b887ae6ad16], but our version has [mower_msgs/Status/33cd1b298baa54308c3c9343754a34fc]. Dropping connection.

@brupje
Copy link

brupje commented Aug 7, 2024

Bonjour, j'ai refait mon ssd, mis ajour les dockers et mis a jour le firmware et toujours aucune remonté, et j'ai ça dans les logs :

I've rebuilt my ssd, updated the dockers and updated the firmware and still no response, and I have this in the logs:
[2024/08/07 11:25:27] [ERROR] subscriber '/mower/status' got an error: Client [/openmower-gui] wants topic /mower/status to have datatype/md5sum [mower_msgs/Status/1e002de26b9c7d2ad33d3b887ae6ad16], but our version has [mower_msgs/Status/33cd1b298baa54308c3c9343754a34fc]. Dropping connection.

Looks like you need to update openmower-gui still. It is using the wrong messages

@juditech3D
Copy link

@brupje une mise a jour via docker compose pull suffit, désolé pour ces question bêtes mais je suis pas un expert.

@brupje an update via docker compose pull is enough, sorry for these stupid questions but I'm not an expert.

@juditech3D
Copy link

juditech3D commented Aug 7, 2024

@brupje merci pour ta réponse rapide, je précise j'ai une 500b et on est une dizaine a être dans le même cas depuis la dernière mise a jour publié et cela même après la mise a jour de firmware comme préconisé.

@brupje thank you for your quick reply, I have a 500b and there are about ten of us in the same situation since the last update was published, even after the firmware update as recommended.

juditech3d@yardforce85:/mowgli-docker $ sudo docker-compose pull
Pulling watchtower ... done
Pulling web ... done
Pulling mosquitto ... done
Pulling roscore ... done
Pulling gui ... done
Pulling rosserial ... done
Pulling openmower ... done
juditech3d@yardforce85:
/mowgli-docker $ sudo docker-compose up -d
mowgli-roscore is up-to-date
mowgli-mqtt is up-to-date
mowgli-docker_watchtower_1 is up-to-date
mowgli-web is up-to-date
mowgli-rosserial is up-to-date
openmower-gui is up-to-date
mowgli-openmower is up-to-date
juditech3d@yardforce85:~/mowgli-docker $

infos : quand je remet ma copie ssd de sauvegarde précédente, elle fonctionne a nouveau juste quelques heures ou jours puis je pense que cela se met automatiquement a jour et me retrouve avec le même problème comme décris ici. Si il y a quelque chose que je peux faire pour faire avancer les choses dites le moi.
J'ai aussi confirmation que la version 500 classic a été affectée et que la mise a jour du firmware regle bien le problème pour cette version.

info: when I put back my previous ssd backup copy, it works again just for a few hours or days then I think it automatically updates and I find myself with the same problem as described here. If there is anything I can do to move things forward let me know. I've also confirmed that the 500 classic version was affected and that the firmware update fixes the problem for this version.

@jeremysalwen
Copy link
Author

Your problem seems to not be with the mowgli firmware, but with the rest of the software you are running, i.e. openmower, openmower-gui. For some reason you are running older versions which are not compatible with the newer ROS message. You need to figure out why this is happening. It is not the fault of your mowgli firmware, and it is not the fault of this PR. You would be getting the same errors if you swapped out for a non-mowgli board.

I suspect you are running some non-standard version of mowgli-docker. Get a fresh checkout from cedbossneo/mowgli-docker, and set it up fresh. I think that will work. Please ask for additional help on the discord server since this is distracting from the discussion of this PR.,

@juditech3D
Copy link

merci @jeremysalwen pour ces éclaircissements et désolé d'avoir polluer ce PR.

thank you @jeremysalwen for these clarifications and sorry to have polluted this PR.

@brupje needed this to compile with his version of platformio.
@jeremysalwen
Copy link
Author

Okay, I have added platformio/framework-stm32cubef4@^1.26.2 to the platform.ini. If someone with 500 classic (non-500b) could test this out, I think we are ready to go.

@wjcloudy
Copy link

THANKS@jeremysalwenfor these clarifications and sorry for polluting this PR.

thank you @jeremysalwen for these clarifications and sorry to have polluted this PR.

Were you ever able to sort this?

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.

8 participants